[PATCH 0/1] MR8163: dmime: Fix use-after-free after performance_CloseDown.
IDirectMusicPerformance8 holds references to music ports in channel_blocks. These ports must outlive their parent IDirectMusic, because in synth_port_Release, they remove themselves from their parent. performance_CloseDown releases the IDirectMusic, but doesn't release its ports. So when these ports are later released in performance_Release, they uses the already freed IDirectMusic. Found by ASan. stacktrace ([post-processed](https://gitlab.winehq.org/yshui/wine-symbolizer)): ``` 016c:0170:err:asan:asan_report ASan: read of 4 bytes at 00007E1C0BFF8518, caller 00006FFFFDE2B9C2 (__asan_report_load4, ../dlls/asan_dynamic_thunk/thunk.c:1020,1) 016c:0170:err:asan:asan_report stacktrace: 016c:0170:err:asan:asan_report 00006FFFFDE2B9C2 (__asan_report_load4, ../dlls/asan_dynamic_thunk/thunk.c:1020,1) 016c:0170:err:asan:asan_report 00006FFFFDE144BC (dmusic_remove_port, ../dlls/dmusic/dmusic.c:312,10) 016c:0170:err:asan:asan_report 00006FFFFDE21A99 (synth_port_Release, ../dlls/dmusic/port.c:129,9) 016c:0170:err:asan:asan_report 00006FFFFE20B88A (channel_block_free, ../dlls/dmime/performance.c:271,28 <- rb_postorder, ../include/wine/rbtree.h:170,46 <- rb_destroy, ../include/wine/rbtree.h:188,19 <- performance_Release, ../dlls/dmime/performance.c:450,5) 016c:0170:err:asan:asan_report 00007FFFFE002FFA (test_COM_audiopath, ../dlls/dmime/tests/dmime.c:972,5 <- func_dmime, ../dlls/dmime/tests/dmime.c:5201,5) 016c:0170:err:asan:asan_report 00007FFFFE082CDD (run_test, ../include/wine/test.h:765,5) 016c:0170:err:asan:asan_report 00007FFFFE082598 (main, ../include/wine/test.h:0,0) 016c:0170:err:asan:asan_report 00007FFFFE08C44F (mainCRTStartup, ../dlls/msvcrt/crt_main.c:60,11) 016c:0170:err:asan:asan_report 00006FFFFF9DD502 (BaseThreadInitThunk, ../dlls/kernel32/thread.c:61,24) 016c:0170:err:asan:asan_report 00006FFFFFC4DC3F (longjmp_regs) 016c:0170:err:asan:asan_report info: 016c:0170:err:asan:asan_report heap-use-after-free, addr 00007E1C0BFF8518 016c:0170:err:asan:asan_report allocated user region: [00007E1C0BFF84F0, 00007E1C0BFF8530) 64 016c:0170:err:asan:asan_report allocated by 19e3 016c:0170:err:asan:asan_report 00006FFFFDE14885 (music_create, ../dlls/dmusic/dmusic.c:587,20) 016c:0170:err:asan:asan_report 00006FFFFDE18853 (ClassFactory_CreateInstance, ../dlls/dmusic/dmusic_main.c:94,9) 016c:0170:err:asan:asan_report 00006FFFFC925ECF (IClassFactory_CreateInstance, include/unknwn.h:245,12 <- CoCreateInstanceEx, ../dlls/combase/combase.c:1890,10) 016c:0170:err:asan:asan_report 00006FFFFC924F61 (CoCreateInstance, ../dlls/combase/combase.c:1679,10) 016c:0170:err:asan:asan_report 00006FFFFE20BE48 (performance_init_dmusic, ../dlls/dmime/performance.c:480,9 <- performance_Init, ../dlls/dmime/performance.c:535,14) 016c:0170:err:asan:asan_report 00006FFFFE211CB9 (performance_InitAudio, ../dlls/dmime/performance.c:1537,9) 016c:0170:err:asan:asan_report 00007FFFFE001949 (test_COM_audiopath, ../dlls/dmime/tests/dmime.c:906,10 <- func_dmime, ../dlls/dmime/tests/dmime.c:5201,5) 016c:0170:err:asan:asan_report 00007FFFFE082CDD (run_test, ../include/wine/test.h:765,5) 016c:0170:err:asan:asan_report 00007FFFFE082598 (main, ../include/wine/test.h:0,0) 016c:0170:err:asan:asan_report 00007FFFFE08C44F (mainCRTStartup, ../dlls/msvcrt/crt_main.c:60,11) 016c:0170:err:asan:asan_report 00006FFFFF9DD502 (BaseThreadInitThunk, ../dlls/kernel32/thread.c:61,24) 016c:0170:err:asan:asan_report 00006FFFFFC4DC3F (longjmp_regs) 016c:0170:err:asan:asan_report freed by 3390 016c:0170:err:asan:asan_report 00006FFFFDE15DC4 (IDirectMusic8Impl_Release, ../dlls/dmusic/dmusic.c:203,5) 016c:0170:err:asan:asan_report 00006FFFFE21158C (performance_CloseDown, ../dlls/dmime/performance.c:1469,9) 016c:0170:err:asan:asan_report 00007FFFFE002FB7 (test_COM_audiopath, ../dlls/dmime/tests/dmime.c:971,5 <- func_dmime, ../dlls/dmime/tests/dmime.c:5201,5) 016c:0170:err:asan:asan_report 00007FFFFE082CDD (run_test, ../include/wine/test.h:765,5) 016c:0170:err:asan:asan_report 00007FFFFE082598 (main, ../include/wine/test.h:0,0) 016c:0170:err:asan:asan_report 00007FFFFE08C44F (mainCRTStartup, ../dlls/msvcrt/crt_main.c:60,11) 016c:0170:err:asan:asan_report 00006FFFFF9DD502 (BaseThreadInitThunk, ../dlls/kernel32/thread.c:61,24) 016c:0170:err:asan:asan_report 00006FFFFFC4DC3F (longjmp_regs) 016c:0170:trace:module:LdrShutdownProcess () ``` -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8163
From: Yuxuan Shui <yshui(a)codeweavers.com> IDirectMusicPerformance8 holds references to music ports in channel_blocks. These ports must outlive their parent IDirectMusic, because in synth_port_Release, they remove themselves from their parent. performance_CloseDown releases the IDirectMusic, but doesn't release its ports. So when these ports are later released in performance_Release, they uses the already freed IDirectMusic. Found by ASan. --- dlls/dmime/performance.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dlls/dmime/performance.c b/dlls/dmime/performance.c index 500bc8b536f..13078056d7e 100644 --- a/dlls/dmime/performance.c +++ b/dlls/dmime/performance.c @@ -447,7 +447,6 @@ static ULONG WINAPI performance_Release(IDirectMusicPerformance8 *iface) TRACE("(%p): ref=%ld\n", This, ref); if (ref == 0) { - wine_rb_destroy(&This->channel_blocks, channel_block_free, NULL); This->safe.DebugInfo->Spare[0] = 0; DeleteCriticalSection(&This->safe); free(This); @@ -1454,6 +1453,7 @@ static HRESULT WINAPI performance_CloseDown(IDirectMusicPerformance8 *iface) performance_set_primary_segment(This, NULL); performance_set_control_segment(This, NULL); + wine_rb_destroy(&This->channel_blocks, channel_block_free, NULL); if (This->master_clock) { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8163
But then aren't we leaking things if CloseDown is not called? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8163#note_104795
On Wed May 28 12:14:06 2025 +0000, Rémi Bernon wrote:
But then aren't we leaking things if CloseDown is not called? which i believe is ok, quoting Microsoft:
Remarks
Failure to call CloseDown can cause memory leaks or program failures.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8163#note_104798
On Wed May 28 12:14:06 2025 +0000, Yuxuan Shui wrote:
which i believe is ok, quoting Microsoft:
Remarks
Failure to call CloseDown can cause memory leaks or program failures. Then we need to make sure it is called in every test before last Release. Also probably a message on destruction if we're leaking things would be nice.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8163#note_104826
participants (3)
-
Rémi Bernon -
Yuxuan Shui
-
Yuxuan Shui (＠yshui)