[Bug 43377] New: valgrind shows an invalid read in dlls/msxml3/tests/ domdoc.c
https://bugs.winehq.org/show_bug.cgi?id=43377 Bug ID: 43377 Summary: valgrind shows an invalid read in dlls/msxml3/tests/domdoc.c Product: Wine Version: 2.12 Hardware: x86 OS: Linux Status: NEW Keywords: download, source, testcase, valgrind Severity: normal Priority: P2 Component: msxml3 Assignee: wine-bugs(a)winehq.org Reporter: austinenglish(a)gmail.com Distribution: Debian ../../../tools/runtest -q -P wine -T ../../.. -M msxml3.dll -p msxml3_test.exe.so domdoc && touch domdoc.ok ==30037== Invalid free() / delete / delete[] / realloc() ==30037== at 0x7BC510DB: notify_free (heap.c:262) ==30037== by 0x7BC556AC: RtlFreeHeap (heap.c:1762) ==30037== by 0x5F97B28: heap_free (msxml_private.h:189) ==30037== by 0x5F9837F: free_properties (domdoc.c:360) ==30037== by 0x5F98CA3: xmldoc_release_refs (domdoc.c:624) ==30037== by 0x5F98CEA: xmldoc_release (domdoc.c:635) ==30037== by 0x5FC36D8: destroy_xmlnode (node.c:1481) ==30037== by 0x5F99C3C: domdoc_Release (domdoc.c:966) ==30037== by 0x4A525C0: IXMLDOMDocument2_Release (msxml2.h:4413) ==30037== by 0x4A525C0: test_get_ownerDocument (???:0) ==30037== by 0x4A76882: func_domdoc (domdoc.c:12609) ==30037== by 0x4AB1624: run_test (test.h:603) ==30037== by 0x4AB1A83: main (test.h:687) ==30037== Address 0x48e0820 is 18 bytes after a recently re-allocated block of size 46 alloc'd ==30037== at 0x7BC51061: notify_alloc (heap.c:254) ==30037== by 0x7BC5554F: RtlAllocateHeap (heap.c:1716) ==30037== by 0x5608BD0: strdupW (freetype.c:1098) ==30037== by 0x560A0ED: load_face (freetype.c:1650) ==30037== by 0x560A862: load_font_list_from_cache (freetype.c:1778) ==30037== by 0x56132B5: WineEngInit (freetype.c:4402) ==30037== by 0x5622222: DllMain (gdiobj.c:677) ==30037== by 0x5641491: __wine_spec_dll_entry (dll_entry.c:40) ==30037== by 0x7BC57861: ??? (loader.c:150) ==30037== by 0x7BC5A291: MODULE_InitDLL (loader.c:1121) ==30037== by 0x7BC5A7E1: process_attach (loader.c:1222) ==30037== by 0x7BC5A728: process_attach (loader.c:1210) ==30037== by 0x7BC5A728: process_attach (loader.c:1210) ==30037== by 0x7BC5A728: process_attach (loader.c:1210) ==30037== by 0x7BC5A728: process_attach (loader.c:1210) ==30037== by 0x7BC5FB42: attach_process_dlls (loader.c:3011) ==30037== by 0x40413EC: ??? (port.c:78) ==30037== ==30037== Invalid free() / delete / delete[] / realloc() ==30037== at 0x7BC510DB: notify_free (heap.c:262) ==30037== by 0x7BC556AC: RtlFreeHeap (heap.c:1762) ==30037== by 0x5F97B28: heap_free (msxml_private.h:189) ==30037== by 0x5F9839F: free_properties (domdoc.c:362) ==30037== by 0x5F98CA3: xmldoc_release_refs (domdoc.c:624) ==30037== by 0x5F98CEA: xmldoc_release (domdoc.c:635) ==30037== by 0x5FC36D8: destroy_xmlnode (node.c:1481) ==30037== by 0x5F99C3C: domdoc_Release (domdoc.c:966) ==30037== by 0x4A525C0: IXMLDOMDocument2_Release (msxml2.h:4413) ==30037== by 0x4A525C0: test_get_ownerDocument (???:0) ==30037== by 0x4A76882: func_domdoc (domdoc.c:12609) ==30037== by 0x4AB1624: run_test (test.h:603) ==30037== by 0x4AB1A83: main (test.h:687) ==30037== Address 0x49065a0 is 4 bytes after a block of size 92 free'd ==30037== at 0x7BC510DB: notify_free (heap.c:262) ==30037== by 0x7BC556AC: RtlFreeHeap (heap.c:1762) ==30037== by 0x55FF13A: FONT_DeleteObject (font.c:814) ==30037== by 0x5622F04: DeleteObject (gdiobj.c:982) ==30037== by 0x532BDF5: get_text_metr_size (sysparams.c:482) ==30037== by 0x532C66E: normalize_nonclientmetrics (sysparams.c:653) ==30037== by 0x532F087: SystemParametersInfoW (sysparams.c:1610) ==30037== by 0x53321D2: GetSystemMetrics (sysparams.c:2459) ==30037== by 0x5331F4F: GetSystemMetrics (sysparams.c:2394) ==30037== by 0x53183B5: NC_AdjustRectOuter (nonclient.c:115) ==30037== by 0x53190E0: NC_HandleNCCalcSize (nonclient.c:414) ==30037== by 0x52CCDB6: DEFWND_DefWinProc (defwnd.c:262) ==30037== by 0x52CEB60: DefWindowProcW (defwnd.c:1023) ==30037== by 0x6DFA03C: notif_wnd_proc (bindprot.c:83) ==30037== by 0x53502FD: ??? (winproc.c:174) ==30037== by 0x535044C: call_window_proc (winproc.c:245) ==30037== by 0x5352800: WINPROC_call_window (winproc.c:901) ==30037== by 0x530DC60: call_window_proc (message.c:2224) ==30037== by 0x5310C82: send_message (message.c:3269) ==30037== by 0x5311413: SendMessageW (message.c:3469) ==30037== Block was alloc'd at ==30037== at 0x7BC51061: notify_alloc (heap.c:254) ==30037== by 0x7BC5554F: RtlAllocateHeap (heap.c:1716) ==30037== by 0x55FE62B: CreateFontIndirectExW (font.c:509) ==30037== by 0x55FE908: CreateFontIndirectW (font.c:559) ==30037== by 0x532BD42: get_text_metr_size (sysparams.c:470) ==30037== by 0x532C66E: normalize_nonclientmetrics (sysparams.c:653) ==30037== by 0x532F087: SystemParametersInfoW (sysparams.c:1610) ==30037== by 0x53321D2: GetSystemMetrics (sysparams.c:2459) ==30037== by 0x5331F4F: GetSystemMetrics (sysparams.c:2394) ==30037== by 0x53183B5: NC_AdjustRectOuter (nonclient.c:115) ==30037== by 0x53190E0: NC_HandleNCCalcSize (nonclient.c:414) ==30037== by 0x52CCDB6: DEFWND_DefWinProc (defwnd.c:262) ==30037== by 0x52CEB60: DefWindowProcW (defwnd.c:1023) ==30037== by 0x6DFA03C: notif_wnd_proc (bindprot.c:83) ==30037== by 0x53502FD: ??? (winproc.c:174) ==30037== by 0x535044C: call_window_proc (winproc.c:245) ==30037== by 0x5352800: WINPROC_call_window (winproc.c:901) ==30037== by 0x530DC60: call_window_proc (message.c:2224) ==30037== by 0x5310C82: send_message (message.c:3269) ==30037== by 0x5311413: SendMessageW (message.c:3469) ==30037== -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=43377 André H. <nerv(a)dawncrow.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |nerv(a)dawncrow.de --- Comment #1 from André H. <nerv(a)dawncrow.de> --- This one seems highly related to the test crash on wine64, which happens in test_get_ownerDocument at the end at IXMLDOMDocument2_Release(doc_owner) which can be traced down to: static void free_properties(domdoc_properties* properties) { if (properties) { if (properties->schemaCache) IXMLDOMSchemaCollection2_Release(properties->schemaCache); ... crash ... because properties is 0x8b400 I get: wine: Unhandled page fault on read access to 0x0008b410 at address 0x8b410 (thread 002a), starting debugger... That's because the exact same properties are already used and freed somewhere else, which should not happen, because there is copy_properties(), but I found a candidate: static HRESULT attach_xmldoc(domdoc *This, xmlDocPtr xml ) { release_namespaces(This); if(This->node.node) { priv_from_xmlDocPtr(get_doc(This))->properties = NULL; if (xmldoc_release(get_doc(This)) != 0) priv_from_xmlDocPtr(get_doc(This))->properties = copy_properties(This->properties); } This->node.node = (xmlNodePtr) xml; if(This->node.node) { xmldoc_add_ref(get_doc(This)); priv_from_xmlDocPtr(get_doc(This))->properties = This->properties; changing this to priv_from_xmlDocPtr(get_doc(This))->properties = copy_properties(This->properties); works fine here, I'll send a patch -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=43377 Serge Gautherie <winehq-bugs_serge_180716(a)gautherie.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |winehq-bugs_serge_180716(a)ga | |utherie.fr -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=43377 --- Comment #2 from Serge Gautherie <winehq-bugs_serge_180716(a)gautherie.fr> --- This bug affects ReactOS test run too: permanent invalid free reported, plus intermittent heap corruption detected. See https://jira.reactos.org/browse/CORE-14579 . Thomas Faber commented: "[Adding copy_properties()] fixes it for me ... but that's probably gonna cause a leak instead. Someone needs to review the lifetime management of those properties objects." -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=43377 François Gouget <fgouget(a)codeweavers.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|valgrind shows an invalid |msxml3:domdoc Crashes in |read in |wow64 due to double free of |dlls/msxml3/tests/domdoc.c |properties CC| |fgouget(a)codeweavers.com -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=43377 --- Comment #3 from François Gouget <fgouget(a)codeweavers.com> --- The crash is pretty systematic in wow64 builds nowadays. (it did succeed for a while probably due to layout changes) https://test.winehq.org/data/patterns.html#msxml3:domdoc -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=43377 François Gouget <fgouget(a)codeweavers.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |RESOLVED Resolution|--- |FIXED --- Comment #4 from François Gouget <fgouget(a)codeweavers.com> --- The msxml3:domdoc crash is fixed: commit 0e87427500b1564ebdaa0a7ad7f627dce2762be9 Author: Francois Gouget <fgouget(a)codeweavers.com> Date: Thu May 6 18:08:06 2021 +0300 msxml3: Refcount the domdoc/xmldoc properties. Multiple domdoc and xmlDoc objects may need to share a common properties object but may be released independently. So add a reference count on the properties object. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=43377 Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com> Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> Signed-off-by: Alexandre Julliard <julliard(a)winehq.org> (unfortunately another bug was introduced in the test a couple of days later that causes it to crash on Windows this time) -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=43377 Gijs Vermeulen <gijsvrm(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Fixed by SHA1| |0e87427500b1564ebdaa0a7ad7f | |627dce2762be9 -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=43377 Serge Gautherie <winehq-bugs_serge_180716(a)gautherie.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- CC|winehq-bugs_serge_180716(a)ga | |utherie.fr | -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=43377 Alexandre Julliard <julliard(a)winehq.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |CLOSED --- Comment #5 from Alexandre Julliard <julliard(a)winehq.org> --- Closing bugs fixed in 6.9. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
participants (2)
-
wine-bugs＠winehq.org -
WineHQ Bugzilla