[PATCH 0/1] MR10437: ntoskrnl.exe/tests: Fix stack use-after-free of OVERLAPPED.
The ioctls started in `test_cancel_thread` complete only after the function has already returned, thus the OVERLAPPED objects needed have become invalid. Make them global variables. This should be fine since we only start the next test thread after we have completed the previously started IO operations. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10437
I don't know, there is probably a better way of fixing this. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10437#note_133547
From: Yuxuan Shui <yshui@codeweavers.com> The ioctls started in `test_cancel_thread` complete only after the function has already returned, thus the OVERLAPPED objects needed have become invalid. Make them global variables. This should be fine since we only start the next test thread after we have completed the previously started IO operations. --- dlls/ntoskrnl.exe/tests/ntoskrnl.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/dlls/ntoskrnl.exe/tests/ntoskrnl.c b/dlls/ntoskrnl.exe/tests/ntoskrnl.c index 06279b1c49e..3eb84da54d0 100644 --- a/dlls/ntoskrnl.exe/tests/ntoskrnl.c +++ b/dlls/ntoskrnl.exe/tests/ntoskrnl.c @@ -455,15 +455,19 @@ struct cancel_thread_ctx enum cancel_test test; }; +static OVERLAPPED o, o2, o3; static DWORD WINAPI test_cancel_thread(void *param) { struct cancel_thread_ctx *ctx = param; NTSTATUS status = STATUS_SUCCESS; IO_STATUS_BLOCK cancel_sb; DWORD cancel_cnt, size; - OVERLAPPED o, o2, o3; BOOL res; + memset(&o, 0, sizeof(o)); + memset(&o2, 0, sizeof(o2)); + memset(&o3, 0, sizeof(o3)); + res = DeviceIoControl(ctx->file, IOCTL_WINETEST_RESET_CANCEL, NULL, 0, NULL, 0, NULL, &o); ok(res, "DeviceIoControl failed: %lu\n", GetLastError()); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10437
Putting it inside cancel_thread_ctx would probably be better. Also, there's no need to touch o3. Also, I don't think there's any guarantee that the ioctls will finish by the time we start the next thread, so we should wait for them in the main thread. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10437#note_133555
participants (3)
-
Elizabeth Figura (＠zfigura) -
Yuxuan Shui
-
Yuxuan Shui (＠yshui)