[PATCH v2 0/2] MR10695: ntdll: Improvements for RtlVirtualUnwind2 with NULL output handler data.
-- v2: ntdll: Catch access violation in RtlVirtualUnwind2() on arm64. ntdll: Do not crash in RtlVirtualUnwind2() in some cases of NULL output parameters on x64. https://gitlab.winehq.org/wine/wine/-/merge_requests/10695
From: Paul Gofman <pgofman@codeweavers.com> --- dlls/ntdll/tests/unwind.c | 46 ++++++++++++++++++++++++++++++++++++++- dlls/ntdll/unwind.c | 10 ++++----- 2 files changed, 50 insertions(+), 6 deletions(-) diff --git a/dlls/ntdll/tests/unwind.c b/dlls/ntdll/tests/unwind.c index 9dea3f3fe60..77d3d1043b9 100644 --- a/dlls/ntdll/tests/unwind.c +++ b/dlls/ntdll/tests/unwind.c @@ -2959,8 +2959,52 @@ static void call_virtual_unwind_x86( int testnum, const struct unwind_test_x86 * if (pRtlVirtualUnwind2) { - CONTEXT new_context = context; + CONTEXT new_context; + + new_context = context; + status = pRtlVirtualUnwind2( UNW_FLAG_NHANDLER, (ULONG_PTR)code_mem, orig_rip, + test->unwind_info ? &runtime_func : NULL, &new_context, + NULL, NULL, &frame, &ctx_ptr, NULL, NULL, NULL, 0 ); + ok( !status, "got %#lx.\n", status ); + new_context = context; + data = (void *)0xdeadbeef; + status = pRtlVirtualUnwind2( UNW_FLAG_NHANDLER, (ULONG_PTR)code_mem, orig_rip, + test->unwind_info ? &runtime_func : NULL, &new_context, + NULL, &data, &frame, &ctx_ptr, NULL, NULL, NULL, 0 ); + ok( !status, "got %#lx.\n", status ); + ok( data == (void *)0xdeadbeef, "got %p.\n", data ); + + new_context = context; + data = (void *)0xdeadbeef; + status = pRtlVirtualUnwind2( UNW_FLAG_EHANDLER, (ULONG_PTR)code_mem, orig_rip, + test->unwind_info ? &runtime_func : NULL, &new_context, + NULL, &data, &frame, &ctx_ptr, NULL, NULL, NULL, 0 ); + ok( !status, "got %#lx.\n", status ); + if (test->results[i].handler) + ok( *(DWORD *)data == 0x08070605, "got %#lx.\n", *(DWORD *)data ); + else + ok( data == (void *)0xdeadbeef, "got %p.\n", data ); + + new_context = context; + handler = (void *)0xdeadbeef; + status = pRtlVirtualUnwind2( UNW_FLAG_NHANDLER, (ULONG_PTR)code_mem, orig_rip, + test->unwind_info ? &runtime_func : NULL, &new_context, + NULL, NULL, &frame, &ctx_ptr, NULL, NULL, &handler, 0 ); + ok( !status, "got %#lx.\n", status ); + ok( !handler, "got %p.\n", handler ); + + new_context = context; + handler = (void *)0xdeadbeef; + data = (void *)0xdeadbeef; + status = pRtlVirtualUnwind2( UNW_FLAG_NHANDLER, (ULONG_PTR)code_mem, orig_rip, + test->unwind_info ? &runtime_func : NULL, &new_context, + NULL, &data, &frame, &ctx_ptr, NULL, NULL, &handler, 0 ); + ok( !status, "got %#lx.\n", status ); + ok( !handler, "got %p.\n", handler ); + ok( data == (void *)0xdeadbeef, "got %p.\n", data ); + + new_context = context; handler = (void *)0xdeadbeef; data = (void *)0xdeadbeef; status = pRtlVirtualUnwind2( UNW_FLAG_EHANDLER, (ULONG_PTR)code_mem, orig_rip, diff --git a/dlls/ntdll/unwind.c b/dlls/ntdll/unwind.c index ace0510f2ef..124e350edd4 100644 --- a/dlls/ntdll/unwind.c +++ b/dlls/ntdll/unwind.c @@ -2071,8 +2071,8 @@ NTSTATUS WINAPI RtlVirtualUnwind2( ULONG type, ULONG_PTR base, ULONG_PTR pc, { context->Rip = *(ULONG64 *)context->Rsp; context->Rsp += sizeof(ULONG64); - *data = NULL; - *handler_ret = NULL; + if (type) *data = NULL; + if (handler_ret) *handler_ret = NULL; return STATUS_SUCCESS; } @@ -2107,7 +2107,7 @@ NTSTATUS WINAPI RtlVirtualUnwind2( ULONG type, ULONG_PTR base, ULONG_PTR pc, TRACE("inside epilog.\n"); interpret_epilog( (BYTE *)pc, context, ctx_ptr ); *frame_ret = info->frame_reg ? context->Rsp - 8 : frame; - *handler_ret = NULL; + if (handler_ret) *handler_ret = NULL; return STATUS_SUCCESS; } } @@ -2188,12 +2188,12 @@ NTSTATUS WINAPI RtlVirtualUnwind2( ULONG type, ULONG_PTR base, ULONG_PTR pc, context->Rsp += sizeof(ULONG64); } - *handler_ret = NULL; + if (handler_ret) *handler_ret = NULL; if (!(info->flags & type)) return STATUS_SUCCESS; /* no matching handler */ if (prolog_offset != ~0) return STATUS_SUCCESS; /* inside prolog */ - *handler_ret = (PEXCEPTION_ROUTINE)((char *)base + handler_data->handler); + if (handler_ret) *handler_ret = (PEXCEPTION_ROUTINE)((char *)base + handler_data->handler); *data = &handler_data->handler + 1; return STATUS_SUCCESS; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10695
From: Paul Gofman <pgofman@codeweavers.com> --- dlls/ntdll/tests/unwind.c | 54 +++++++++++++++++++++++++++++++++++++++ dlls/ntdll/unwind.c | 39 +++++++++++++++++++++------- 2 files changed, 84 insertions(+), 9 deletions(-) diff --git a/dlls/ntdll/tests/unwind.c b/dlls/ntdll/tests/unwind.c index 77d3d1043b9..55a8d353336 100644 --- a/dlls/ntdll/tests/unwind.c +++ b/dlls/ntdll/tests/unwind.c @@ -1649,6 +1649,60 @@ static void call_virtual_unwind_arm64( void *code_mem, int testnum, const struct if (pRtlVirtualUnwind2) { + if (test->unwind_info) + { + /* EXCEPTION_ACCESS_VIOLATION is thrown on Windows with NULL handler_data if there is no runtime function. */ + new_context = context; + frame = 0xdeadbeef; + status = pRtlVirtualUnwind2( UNW_FLAG_NHANDLER, (ULONG_PTR)code_mem, orig_pc, + (RUNTIME_FUNCTION *)&runtime_func, + (CONTEXT *)&new_context, NULL, NULL, + &frame, &ctx_ptr, NULL, NULL, NULL, 0 ); + ok( status == STATUS_ACCESS_VIOLATION, "got %#lx\n", status ); + ok( frame == (test->results[i].frame_offset ? (ULONG64)fake_stack : 0) + test->results[i].frame, "wrong frame %p/%p\n", + (void *)frame, (char *)(test->results[i].frame_offset ? fake_stack : NULL) + test->results[i].frame ); + + new_context = context; + handler = (void *)0xdeadbeef; + frame = 0xdeadbeef; + status = pRtlVirtualUnwind2( UNW_FLAG_NHANDLER, (ULONG_PTR)code_mem, orig_pc, + (RUNTIME_FUNCTION *)&runtime_func, + (CONTEXT *)&new_context, NULL, NULL, + &frame, &ctx_ptr, NULL, NULL, &handler, 0 ); + ok( status == STATUS_ACCESS_VIOLATION, "got %#lx\n", status ); + ok( frame == (test->results[i].frame_offset ? (ULONG64)fake_stack : 0) + test->results[i].frame, "wrong frame %p/%p\n", + (void *)frame, (char *)(test->results[i].frame_offset ? fake_stack : NULL) + test->results[i].frame ); + if (test->results[i].handler > 0) + ok( (char *)handler == (char *)code_mem + 0x200, "got %p\n", handler ); + else + ok( !handler, "got %p\n", handler ); + } + + new_context = context; + data = (void *)0xdeadbeef; + frame = 0xdeadbeef; + status = pRtlVirtualUnwind2( UNW_FLAG_NHANDLER, (ULONG_PTR)code_mem, orig_pc, + test->unwind_info ? (RUNTIME_FUNCTION *)&runtime_func : NULL, + (CONTEXT *)&new_context, NULL, &data, + &frame, &ctx_ptr, NULL, NULL, NULL, 0 ); + ok( frame == (test->results[i].frame_offset ? (ULONG64)fake_stack : 0) + test->results[i].frame, "wrong frame %p/%p\n", + (void *)frame, (char *)(test->results[i].frame_offset ? fake_stack : NULL) + test->results[i].frame ); + if (runtime_func.Flag && test->results[i].handler > 0) + { + ok( status == STATUS_ACCESS_VIOLATION, "got %#lx.\n", status ); + ok( data == (void *)0xdeadbeef, "got %p.\n", data ); + } + else if (test->results[i].handler < -1) + { + ok( status == STATUS_BAD_FUNCTION_TABLE, "RtlVirtualUnwind2 failed %lx\n", status ); + ok( data == (void *)0xdeadbeef, "handler data set to %p\n", data ); + } + else + { + ok( !status, "got %#lx\n", status ); + ok( data != (void *)0xdeadbeef, "got %p.\n", data ); + } + new_context = context; handler = (void *)0xdeadbeef; data = (void *)0xdeadbeef; diff --git a/dlls/ntdll/unwind.c b/dlls/ntdll/unwind.c index 124e350edd4..5b79e48ad85 100644 --- a/dlls/ntdll/unwind.c +++ b/dlls/ntdll/unwind.c @@ -794,25 +794,46 @@ NTSTATUS WINAPI RtlVirtualUnwind2( ULONG type, ULONG_PTR base, ULONG_PTR pc, PEXCEPTION_ROUTINE *handler_ret, ULONG flags ) { BOOLEAN final_pc_from_lr = TRUE; + PEXCEPTION_ROUTINE handler; + void *data = NULL; + TRACE( "type %lx base %I64x pc %I64x rva %I64x sp %I64x\n", type, base, pc, pc - base, context->Sp ); if (limit_low || limit_high) FIXME( "limits not supported\n" ); if (!func && pc == context->Lr) return STATUS_BAD_FUNCTION_TABLE; /* invalid leaf function */ - *handler_data = NULL; context->ContextFlags |= CONTEXT_UNWOUND_TO_CALL; if (!func) /* leaf function */ - *handler_ret = NULL; - else if (func->Flag) - *handler_ret = unwind_packed_data( base, pc, func, context, ctx_ptr ); - else - *handler_ret = unwind_full_data( base, pc, func, context, handler_data, ctx_ptr, &final_pc_from_lr ); + { + context->Pc = context->Lr; + *frame_ret = context->Sp; + if (handler_ret) *handler_ret = NULL; + *handler_data = NULL; + return STATUS_SUCCESS; + } - if (final_pc_from_lr) context->Pc = context->Lr; + __TRY + { + if (func->Flag) + handler = unwind_packed_data( base, pc, func, context, ctx_ptr ); + else + handler = unwind_full_data( base, pc, func, context, &data, ctx_ptr, &final_pc_from_lr ); - TRACE( "ret: pc=%I64x lr=%I64x sp=%I64x handler=%p\n", context->Pc, context->Lr, context->Sp, *handler_ret ); - *frame_ret = context->Sp; + if (final_pc_from_lr) context->Pc = context->Lr; + *frame_ret = context->Sp; + + if (handler_ret) *handler_ret = handler; + *handler_data = data; + } + __EXCEPT_PAGE_FAULT + { + WARN( "Access violation.\n" ); + return STATUS_ACCESS_VIOLATION; + } + __ENDTRY + + TRACE( "ret: pc=%I64x lr=%I64x sp=%I64x handler=%p\n", context->Pc, context->Lr, context->Sp, handler ); return STATUS_SUCCESS; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10695
v2: * remove one unneeded change in the first patch. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10695#note_137024
participants (2)
-
Paul Gofman -
Paul Gofman (＠gofman)