[PATCH 0/1] MR10197: mmdevapi/tests: Use temporary during reallocation (ASan).
Followup of d7e358c7. CC: @giomasce It seems `base_fmt` points to memory inside the `wave_formats`, which may be no longer valid after reallocation. <details> <summary>ASan details, [gitlab run](https://gitlab.winehq.org/bernhardu/wine/-/jobs/231755#L2473)</summary> ``` ==mmdevapi_test.exe==848==ERROR: AddressSanitizer: heap-use-after-free on address 0x02400110 at pc 0x78654532 bp 0x0022f9e0 sp 0x0022f5b0 READ of size 40 at 0x02400110 thread T0 #0 0x78654531 in __asan_memcpy /home/runner/work/llvm-mingw/llvm-mingw/llvm-project/compiler-rt/lib/asan/asan_interceptors_memintrinsics.cpp:65:3 #1 0x00416bfc in push_wave_format_with_context /builds/bernhardu/wine/build64/../dlls/mmdevapi/tests/render.c:526:46 #2 0x00416b75 in repush_wave_format_as_unextensible /builds/bernhardu/wine/build64/../dlls/mmdevapi/tests/render.c:550:11 #3 0x00416041 in fill_wave_formats /builds/bernhardu/wine/build64/../dlls/mmdevapi/tests/render.c:705:5 #4 0x004181f3 in test_audioclient /builds/bernhardu/wine/build64/../dlls/mmdevapi/tests/render.c:260:9 #5 0x0041703d in func_render /builds/bernhardu/wine/build64/../dlls/mmdevapi/tests/render.c:3211:5 #6 0x004340a1 in run_test /builds/bernhardu/wine/build64/../include/wine/test.h:780:5 #7 0x00433b5a in main /builds/bernhardu/wine/build64/../include/wine/test.h:900:12 #8 0x004359a5 in mainCRTStartup /builds/bernhardu/wine/build64/../dlls/msvcrt/crt_main.c:62:11 #9 0x7bcd367f in BaseThreadInitThunk (C:\windows\system32\kernel32.dll+0x7b82367f) #10 0x7be4677a in call_thread_func_wrapper (C:\windows\system32\ntdll.dll+0x7bc4677a) #11 0x7be470ea in call_thread_func /builds/bernhardu/wine/build64/../dlls/ntdll/signal_i386.c:503:9 0x02400110 is located 0 bytes inside of 44-byte region [0x02400110,0x0240013c) freed by thread T0 here: #0 0x78656013 in __asan::SharedReAlloc(void* (void*, unsigned long, void*, unsigned int) stdcall*, unsigned int (void*, unsigned long, void*) stdcall*, int (void*, unsigned long, void*) stdcall*, void* (void*, unsigned long, unsigned int) stdcall*, void*, unsigned long, void*, unsigned int) /home/runner/work/llvm-mingw/llvm-mingw/llvm-project/compiler-rt/lib/asan/asan_malloc_win.cpp:276:3 #1 0x786563f8 in HeapReAlloc /home/runner/work/llvm-mingw/llvm-mingw/llvm-project/compiler-rt/lib/asan/asan_malloc_win.cpp:408:10 #2 0x79ddf67a in realloc /builds/bernhardu/wine/build64/../dlls/msvcrt/heap.c:462:20 #3 0x00416bd7 in push_wave_format_with_context /builds/bernhardu/wine/build64/../dlls/mmdevapi/tests/render.c:521:24 #4 0x00416b75 in repush_wave_format_as_unextensible /builds/bernhardu/wine/build64/../dlls/mmdevapi/tests/render.c:550:11 #5 0x00416041 in fill_wave_formats /builds/bernhardu/wine/build64/../dlls/mmdevapi/tests/render.c:705:5 #6 0x004181f3 in test_audioclient /builds/bernhardu/wine/build64/../dlls/mmdevapi/tests/render.c:260:9 #7 0x0041703d in func_render /builds/bernhardu/wine/build64/../dlls/mmdevapi/tests/render.c:3211:5 #8 0x004340a1 in run_test /builds/bernhardu/wine/build64/../include/wine/test.h:780:5 #9 0x00433b5a in main /builds/bernhardu/wine/build64/../include/wine/test.h:900:12 #10 0x004359a5 in mainCRTStartup /builds/bernhardu/wine/build64/../dlls/msvcrt/crt_main.c:62:11 #11 0x7bcd367f in BaseThreadInitThunk (C:\windows\system32\kernel32.dll+0x7b82367f) #12 0x7be4677a in call_thread_func_wrapper (C:\windows\system32\ntdll.dll+0x7bc4677a) #13 0x7be470ea in call_thread_func /builds/bernhardu/wine/build64/../dlls/ntdll/signal_i386.c:503:9 previously allocated by thread T0 here: #0 0x78655d76 in HeapAlloc /home/runner/work/llvm-mingw/llvm-mingw/llvm-project/compiler-rt/lib/asan/asan_malloc_win.cpp:237:3 #1 0x79ddeedc in msvcrt_heap_alloc /builds/bernhardu/wine/build64/../dlls/msvcrt/heap.c:71:12 #2 0x79ddf618 in malloc /builds/bernhardu/wine/build64/../dlls/msvcrt/heap.c:436:15 #3 0x79ddf682 in realloc /builds/bernhardu/wine/build64/../dlls/msvcrt/heap.c:461:20 #4 0x00416bd7 in push_wave_format_with_context /builds/bernhardu/wine/build64/../dlls/mmdevapi/tests/render.c:521:24 #5 0x00416b2b in push_wave_format /builds/bernhardu/wine/build64/../dlls/mmdevapi/tests/render.c:534:12 #6 0x0041603c in fill_wave_formats /builds/bernhardu/wine/build64/../dlls/mmdevapi/tests/render.c:704:5 #7 0x004181f3 in test_audioclient /builds/bernhardu/wine/build64/../dlls/mmdevapi/tests/render.c:260:9 #8 0x0041703d in func_render /builds/bernhardu/wine/build64/../dlls/mmdevapi/tests/render.c:3211:5 #9 0x004340a1 in run_test /builds/bernhardu/wine/build64/../include/wine/test.h:780:5 #10 0x00433b5a in main /builds/bernhardu/wine/build64/../include/wine/test.h:900:12 #11 0x004359a5 in mainCRTStartup /builds/bernhardu/wine/build64/../dlls/msvcrt/crt_main.c:62:11 #12 0x7bcd367f in BaseThreadInitThunk (C:\windows\system32\kernel32.dll+0x7b82367f) #13 0x7be4677a in call_thread_func_wrapper (C:\windows\system32\ntdll.dll+0x7bc4677a) #14 0x7be470ea in call_thread_func /builds/bernhardu/wine/build64/../dlls/ntdll/signal_i386.c:503:9 SUMMARY: AddressSanitizer: heap-use-after-free /builds/bernhardu/wine/build64/../dlls/mmdevapi/tests/render.c:526:46 in push_wave_format_with_context``` </details> -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10197
From: Bernhard Übelacker <bernhardu@mailbox.org> Followup of d7e358c7. --- dlls/mmdevapi/tests/render.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/dlls/mmdevapi/tests/render.c b/dlls/mmdevapi/tests/render.c index 799b6ccfffb..790a33a73a8 100644 --- a/dlls/mmdevapi/tests/render.c +++ b/dlls/mmdevapi/tests/render.c @@ -516,14 +516,22 @@ static WAVEFORMATEXTENSIBLE *push_wave_format_with_context(const WAVEFORMATEXTEN { if (wave_format_count == wave_format_capacity) { + /* Variable base_fmt may point inside wave_formats memory, + * therefore use a temporary during reallocation. */ + WAVEFORMATEXTENSIBLE tmp_fmt; + tmp_fmt = *base_fmt; + wave_format_capacity = max(1, 2 * wave_format_capacity); wave_formats = realloc(wave_formats, sizeof(*wave_formats) * wave_format_capacity); assert(wave_formats); + + wave_formats[wave_format_count].format = tmp_fmt; } + else + wave_formats[wave_format_count].format = *base_fmt; - wave_formats[wave_format_count].format = *base_fmt; wave_formats[wave_format_count].additional_context = additional_context; return &wave_formats[wave_format_count++].format; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10197
Good point, thanks for debugging this! -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10197#note_130586
This merge request was approved by Giovanni Mascellani. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10197
This merge request was approved by Huw Davies. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10197
participants (4)
-
Bernhard Übelacker -
Bernhard Übelacker (＠bernhardu)
-
Giovanni Mascellani (＠giomasce)
-
Huw Davies (＠huw)