Re: [PATCH] ntdll: Set Rip in for longjmp in RtlRestoreContext
On 11.08.2016 19:27, Daniel Lehman wrote:
Some tests will fail but are unrelated to this patch: https://urldefense.proofpoint.com/v2/url?u=http-3A__test.winehq.org_data_882... https://urldefense.proofpoint.com/v2/url?u=http-3A__test.winehq.org_data_882... https://urldefense.proofpoint.com/v2/url?u=http-3A__test.winehq.org_data_882...
0001-ntdll-Set-Rip-in-for-longjmp-in-RtlRestoreContext.txt
From eaab70ee4ddc866526df0341853ccdb599cf7674 Mon Sep 17 00:00:00 2001 From: Daniel Lehman <dlehman(a)esri.com> Date: Mon, 1 Aug 2016 14:52:32 -0700 Subject: [PATCH] ntdll: Set Rip in for longjmp in RtlRestoreContext
Signed-off-by: Daniel Lehman <dlehman(a)esri.com> --- dlls/ntdll/signal_x86_64.c | 1 + dlls/ntdll/tests/exception.c | 202 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 203 insertions(+)
diff --git a/dlls/ntdll/signal_x86_64.c b/dlls/ntdll/signal_x86_64.c index 4c88536..598e08a 100644 --- a/dlls/ntdll/signal_x86_64.c +++ b/dlls/ntdll/signal_x86_64.c @@ -3513,6 +3513,7 @@ void WINAPI RtlRestoreContext( CONTEXT *context, EXCEPTION_RECORD *rec ) context->R13 = jmp->R13; context->R14 = jmp->R14; context->R15 = jmp->R15; + context->Rip = jmp->Rip; context->u.s.Xmm6 = jmp->Xmm6; context->u.s.Xmm7 = jmp->Xmm7; context->u.s.Xmm8 = jmp->Xmm8;
Please note that this also modifies the behavior of RtlUnwindEx (which gets the target_ip passed directly as a function argument). With your change it will be ignored, which might need additional (at least manual) tests to make sure this is correct.
diff --git a/dlls/ntdll/tests/exception.c b/dlls/ntdll/tests/exception.c index a086436..4846769 100644 --- a/dlls/ntdll/tests/exception.c +++ b/dlls/ntdll/tests/exception.c @@ -53,10 +53,44 @@ static NTSTATUS (WINAPI *pNtSetInformationProcess)(HANDLE, PROCESSINFOCLASS, PV static BOOL (WINAPI *pIsWow64Process)(HANDLE, PBOOL);
#if defined(__x86_64__) +typedef struct _SETJMP_FLOAT128 +{ + unsigned __int64 DECLSPEC_ALIGN(16) Part[2]; +} SETJMP_FLOAT128; + +typedef struct _JUMP_BUFFER +{ + unsigned __int64 Frame; + unsigned __int64 Rbx; + unsigned __int64 Rsp; + unsigned __int64 Rbp; + unsigned __int64 Rsi; + unsigned __int64 Rdi; + unsigned __int64 R12; + unsigned __int64 R13; + unsigned __int64 R14; + unsigned __int64 R15; + unsigned __int64 Rip; + unsigned __int64 Spare; + SETJMP_FLOAT128 Xmm6; + SETJMP_FLOAT128 Xmm7; + SETJMP_FLOAT128 Xmm8; + SETJMP_FLOAT128 Xmm9; + SETJMP_FLOAT128 Xmm10; + SETJMP_FLOAT128 Xmm11; + SETJMP_FLOAT128 Xmm12; + SETJMP_FLOAT128 Xmm13; + SETJMP_FLOAT128 Xmm14; + SETJMP_FLOAT128 Xmm15; +} _JUMP_BUFFER; + static BOOLEAN (CDECL *pRtlAddFunctionTable)(RUNTIME_FUNCTION*, DWORD, DWORD64); static BOOLEAN (CDECL *pRtlDeleteFunctionTable)(RUNTIME_FUNCTION*); static BOOLEAN (CDECL *pRtlInstallFunctionTableCallback)(DWORD64, DWORD64, DWORD, PGET_RUNTIME_FUNCTION_CALLBACK, PVOID, PCWSTR); static PRUNTIME_FUNCTION (WINAPI *pRtlLookupFunctionEntry)(ULONG64, ULONG64*, UNWIND_HISTORY_TABLE*); +static VOID (WINAPI *pRtlCaptureContext)(CONTEXT*); +static VOID (CDECL *pRtlRestoreContext)(CONTEXT*, EXCEPTION_RECORD*); +static int (CDECL *p_setjmp)(_JUMP_BUFFER*); #endif
#ifdef __i386__ @@ -1631,6 +1665,164 @@ static void test_virtual_unwind(void) call_virtual_unwind( i, &tests[i] ); }
+static int consolidate_dummy_called; +static PVOID CALLBACK test_consolidate_dummy(EXCEPTION_RECORD *rec) +{ + CONTEXT *ctx = (CONTEXT *)rec->ExceptionInformation[1]; + consolidate_dummy_called = 1; + ok(ctx->Rip == 0xdeadbeef, "test_consolidate_dummy failed for Rip, expected: 0xdeadbeef, got: %lx\n", ctx->Rip); + return (PVOID)rec->ExceptionInformation[2]; +} + +static void test_restore_context(void) +{ + XMM_SAVE_AREA32 *fltsave; + EXCEPTION_RECORD rec; + volatile int done; /* gcc 4.5.2 rearranges 'done' and RtlCaptureContext */ + volatile int pass;
A better way to make sure the compiler does not rearrange the code would be to use Interlocked*() functions.
+ _JUMP_BUFFER buf; + CONTEXT ctx; + + if (!pRtlRestoreContext || !pRtlCaptureContext || !p_setjmp) + { + skip("RtlCaptureContext/RtlRestoreContext/_setjmp not found\n"); + return; + } + + /* RtlRestoreContext(NULL, NULL); crashes on Windows */ + + /* test simple case of capture and restore context */ + pass = 0; + done = 0; + pRtlCaptureContext(&ctx); + switch (pass) + { + case 0: + ok(!done, "done on first pass\n"); + break; + case 1: + ok(done, "not done on second pass\n"); + break; + default: + ok(0, "bad pass = %i done = %i\n", pass, done); + break; + } + ++pass; + if (done) goto test_longjmp; + done = 1;
I don't think the second variable adds much value here. A simplified version like below would test the same things: pass = 0; pRtlCaptureContext(&ctx); ok(pass < 2, "unexpected pass = %d\n", pass); if (!pass++) pRtlRestoreContext(&ctx, NULL);
+ + pRtlRestoreContext(&ctx, NULL); + +test_longjmp:
I do not see any need to use goto here, regular if () checks would also do the job.
+ fltsave = (XMM_SAVE_AREA32 *)((char*)&ctx + 0x100);
It is better to avoid such hardcoded constants. Why not access the field directly?
+ done = 0; + pRtlCaptureContext(&ctx);
What is the purpose of the RtlCaptureContext call here? The result does not seem to be used. After RtlRestoreContext() the ctx struct has been filled with the jump buffer values.
+ p_setjmp(&buf); + if (done) + { + ok(buf.Rbx == ctx.Rbx, "RtlRestoreContext:longjump failed for Rbx, expected: %lx, got: %lx\n", buf.Rbx, ctx.Rbx); + ok(buf.Rsp == ctx.Rsp, "RtlRestoreContext:longjump failed for Rsp, expected: %lx, got: %lx\n", buf.Rsp, ctx.Rsp); + ok(buf.Rbp == ctx.Rbp, "RtlRestoreContext:longjump failed for Rbp, expected: %lx, got: %lx\n", buf.Rbp, ctx.Rbp); + ok(buf.Rsi == ctx.Rsi, "RtlRestoreContext:longjump failed for Rsi, expected: %lx, got: %lx\n", buf.Rsi, ctx.Rsi); + ok(buf.Rdi == ctx.Rdi, "RtlRestoreContext:longjump failed for Rdi, expected: %lx, got: %lx\n", buf.Rdi, ctx.Rdi); + ok(buf.R12 == ctx.R12, "RtlRestoreContext:longjump failed for R12, expected: %lx, got: %lx\n", buf.R12, ctx.R12); + ok(buf.R13 == ctx.R13, "RtlRestoreContext:longjump failed for R13, expected: %lx, got: %lx\n", buf.R13, ctx.R13); + ok(buf.R14 == ctx.R14, "RtlRestoreContext:longjump failed for R14, expected: %lx, got: %lx\n", buf.R14, ctx.R14); + ok(buf.R15 == ctx.R15, "RtlRestoreContext:longjump failed for R15, expected: %lx, got: %lx\n", buf.R15, ctx.R15); + + ok(buf.Xmm6.Part[1] == fltsave->XmmRegisters[6].High && buf.Xmm6.Part[0] == fltsave->XmmRegisters[6].Low, + "RtlRestoreContext:longjump failed for Xmm6, expected: %lx%08lx, got: %lx%08lx\n", + buf.Xmm6.Part[1], buf.Xmm6.Part[0], fltsave->XmmRegisters[6].High, fltsave->XmmRegisters[6].Low); + ok(buf.Xmm7.Part[1] == fltsave->XmmRegisters[7].High && buf.Xmm7.Part[0] == fltsave->XmmRegisters[7].Low, + "RtlRestoreContext:longjump failed for Xmm7, expected: %lx%08lx, got: %lx%08lx\n", + buf.Xmm7.Part[1], buf.Xmm7.Part[0], fltsave->XmmRegisters[7].High, fltsave->XmmRegisters[7].Low); + ok(buf.Xmm8.Part[1] == fltsave->XmmRegisters[8].High && buf.Xmm8.Part[0] == fltsave->XmmRegisters[8].Low, + "RtlRestoreContext:longjump failed for Xmm8, expected: %lx%08lx, got: %lx%08lx\n", + buf.Xmm8.Part[1], buf.Xmm8.Part[0], fltsave->XmmRegisters[8].High, fltsave->XmmRegisters[8].Low); + ok(buf.Xmm9.Part[1] == fltsave->XmmRegisters[9].High && buf.Xmm9.Part[0] == fltsave->XmmRegisters[9].Low, + "RtlRestoreContext:longjump failed for Xmm9, expected: %lx%08lx, got: %lx%08lx\n", + buf.Xmm9.Part[1], buf.Xmm9.Part[0], fltsave->XmmRegisters[9].High, fltsave->XmmRegisters[9].Low); + ok(buf.Xmm10.Part[1] == fltsave->XmmRegisters[10].High && buf.Xmm10.Part[0] == fltsave->XmmRegisters[10].Low, + "RtlRestoreContext:longjump failed for Xmm10, expected: %lx%08lx, got: %lx%08lx\n", + buf.Xmm10.Part[1], buf.Xmm10.Part[0], fltsave->XmmRegisters[10].High, fltsave->XmmRegisters[10].Low); + ok(buf.Xmm11.Part[1] == fltsave->XmmRegisters[11].High && buf.Xmm11.Part[0] == fltsave->XmmRegisters[11].Low, + "RtlRestoreContext:longjump failed for Xmm11, expected: %lx%08lx, got: %lx%08lx\n", + buf.Xmm11.Part[1], buf.Xmm11.Part[0], fltsave->XmmRegisters[11].High, fltsave->XmmRegisters[11].Low); + ok(buf.Xmm12.Part[1] == fltsave->XmmRegisters[12].High && buf.Xmm12.Part[0] == fltsave->XmmRegisters[12].Low, + "RtlRestoreContext:longjump failed for Xmm12, expected: %lx%08lx, got: %lx%08lx\n", + buf.Xmm12.Part[1], buf.Xmm12.Part[0], fltsave->XmmRegisters[12].High, fltsave->XmmRegisters[12].Low); + ok(buf.Xmm13.Part[1] == fltsave->XmmRegisters[13].High && buf.Xmm13.Part[0] == fltsave->XmmRegisters[13].Low, + "RtlRestoreContext:longjump failed for Xmm13, expected: %lx%08lx, got: %lx%08lx\n", + buf.Xmm13.Part[1], buf.Xmm13.Part[0], fltsave->XmmRegisters[13].High, fltsave->XmmRegisters[13].Low); + ok(buf.Xmm14.Part[1] == fltsave->XmmRegisters[14].High && buf.Xmm14.Part[0] == fltsave->XmmRegisters[14].Low, + "RtlRestoreContext:longjump failed for Xmm14, expected: %lx%08lx, got: %lx%08lx\n", + buf.Xmm14.Part[1], buf.Xmm14.Part[0], fltsave->XmmRegisters[14].High, fltsave->XmmRegisters[14].Low); + ok(buf.Xmm15.Part[1] == fltsave->XmmRegisters[15].High && buf.Xmm15.Part[0] == fltsave->XmmRegisters[15].Low, + "RtlRestoreContext:longjump failed for Xmm15, expected: %lx%08lx, got: %lx%08lx\n", + buf.Xmm15.Part[1], buf.Xmm15.Part[0], fltsave->XmmRegisters[15].High, fltsave->XmmRegisters[15].Low); + + goto test_consolidate;
Similar to above, it would be better to avoid goto. All the code below could just be moved to an else branch.
+ } + done = 1; + + rec.ExceptionCode = STATUS_LONGJUMP; + rec.NumberParameters = 1; + rec.ExceptionInformation[0] = (DWORD64)&buf; + + ctx.Rbx = 0xdeadbeef; + ctx.Rsp = 0xdeadbeef; + ctx.Rbp = 0xdeadbeef; + ctx.Rsi = 0xdeadbeef; + ctx.Rdi = 0xdeadbeef; + ctx.R12 = 0xdeadbeef; + ctx.R13 = 0xdeadbeef; + ctx.R14 = 0xdeadbeef; + ctx.R15 = 0xdeadbeef; + + fltsave->XmmRegisters[6].High = 0xbaadf00d; + fltsave->XmmRegisters[6].Low = 0xdeadbeef; + fltsave->XmmRegisters[7].High = 0xbaadf00d; + fltsave->XmmRegisters[7].Low = 0xdeadbeef; + fltsave->XmmRegisters[8].High = 0xbaadf00d; + fltsave->XmmRegisters[8].Low = 0xdeadbeef; + fltsave->XmmRegisters[9].High = 0xbaadf00d; + fltsave->XmmRegisters[9].Low = 0xdeadbeef; + fltsave->XmmRegisters[10].High = 0xbaadf00d; + fltsave->XmmRegisters[10].Low = 0xdeadbeef; + fltsave->XmmRegisters[11].High = 0xbaadf00d; + fltsave->XmmRegisters[11].Low = 0xdeadbeef; + fltsave->XmmRegisters[12].High = 0xbaadf00d; + fltsave->XmmRegisters[12].Low = 0xdeadbeef; + fltsave->XmmRegisters[13].High = 0xbaadf00d; + fltsave->XmmRegisters[13].Low = 0xdeadbeef; + fltsave->XmmRegisters[14].High = 0xbaadf00d; + fltsave->XmmRegisters[14].Low = 0xdeadbeef; + fltsave->XmmRegisters[15].High = 0xbaadf00d; + fltsave->XmmRegisters[15].Low = 0xdeadbeef;
It would look better if you would use a loop or just a memset() for something like that.
+ + pRtlRestoreContext(&ctx, &rec); + +test_consolidate: + done = 0; + + pRtlCaptureContext(&ctx); + if (done) + { + ok(consolidate_dummy_called, "test_consolidate_dummy not called\n"); + return; + } + done = 1; + + rec.ExceptionCode = STATUS_UNWIND_CONSOLIDATE; + rec.NumberParameters = 3; + rec.ExceptionInformation[0] = (DWORD64)test_consolidate_dummy; + rec.ExceptionInformation[1] = (DWORD64)&ctx; + rec.ExceptionInformation[2] = ctx.Rip; + ctx.Rip = 0xdeadbeef; + + pRtlRestoreContext(&ctx, &rec); +} + static RUNTIME_FUNCTION* CALLBACK dynamic_unwind_callback( DWORD64 pc, PVOID context ) { static const int code_offset = 1024; @@ -2155,6 +2347,9 @@ static void test_vectored_continue_handler(void) START_TEST(exception) { HMODULE hntdll = GetModuleHandleA("ntdll.dll"); +#if defined(__x86_64__) + HMODULE hmsvcrt = LoadLibraryA("msvcrt.dll"); +#endif
code_mem = VirtualAlloc(NULL, 65536, MEM_RESERVE | MEM_COMMIT, PAGE_EXECUTE_READWRITE); if(!code_mem) { @@ -2267,6 +2462,12 @@ START_TEST(exception) "RtlInstallFunctionTableCallback" ); pRtlLookupFunctionEntry = (void *)GetProcAddress( hntdll, "RtlLookupFunctionEntry" ); + pRtlCaptureContext = (void *)GetProcAddress( hntdll, + "RtlCaptureContext" ); + pRtlRestoreContext = (void *)GetProcAddress( hntdll, + "RtlRestoreContext" ); + p_setjmp = (void *)GetProcAddress( hmsvcrt, + "_setjmp" );
test_debug_registers(); test_outputdebugstring(1, FALSE); @@ -2275,6 +2476,7 @@ START_TEST(exception) test_breakpoint(1); test_vectored_continue_handler(); test_virtual_unwind(); + test_restore_context();
if (pRtlAddFunctionTable && pRtlDeleteFunctionTable && pRtlInstallFunctionTableCallback && pRtlLookupFunctionEntry) test_dynamic_unwind(); -- 1.9.5
participants (1)
-
Sebastian Lackner