Skip to content

Commit

Permalink
Fix nonvolatile context restoration (#101709)
Browse files Browse the repository at this point in the history
* Fix nonvolatile context restoration

There is a possibility of a race between the
ClrRestoreNonVolatileContext and an async signal handling (like
the one we use for runtime suspension). If the signal kicks in after
we've loaded Rsp, but before we jumped to the target address, the
context we are loading the registers from could get overwritten by the
signal handler stack. So the ClrRestoreNonVolatileContext would end up
jumping into a wrong target address.

The fix is to load the target address into a register before loading the
Rsp and then jumping using the register.

* Fix arm and x86
  • Loading branch information
janvorli authored Apr 30, 2024
1 parent 30b3721 commit 7aefd27
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 16 deletions.
21 changes: 12 additions & 9 deletions src/coreclr/pal/src/arch/arm/context2.S
Original file line number Diff line number Diff line change
Expand Up @@ -160,25 +160,28 @@ LOCAL_LABEL(Restore_CONTEXT_FLOATING_POINT):
ldr R2, [r0, #(CONTEXT_Cpsr)]
msr APSR, r2

// Ideally, we would like to use `ldmia r0, {r0-r12, sp, lr, pc}` here,
// but clang 3.6 and later, as per ARM recommendation, disallows using
// Sp in the register list, and Pc and Lr simultaneously.
// So we are going to use the IPC register r12 to copy Sp, Lr and Pc
// which should be ok -- TODO: Is this really ok?
ldr r1, [r0, #(CONTEXT_Sp)]
ldr r2, [r0, #(CONTEXT_Pc)]
str r2, [r1, #-4]
ldr r2, [r0, #(CONTEXT_R12)]
str r2, [r1, #-8]
add r12, r0, CONTEXT_R0
ldm r12, {r0-r11}
ldr sp, [r12, #(CONTEXT_Sp - (CONTEXT_R0))]
ldr lr, [r12, #(CONTEXT_Lr - (CONTEXT_R0))]
ldr pc, [r12, #(CONTEXT_Pc - (CONTEXT_R0))]
ldr r12, [r12, #(CONTEXT_Sp - (CONTEXT_R0))]
sub r12, r12, #8
mov sp, r12
pop {r12, pc}

LOCAL_LABEL(No_Restore_CONTEXT_INTEGER):

ldr r2, [r0, #(CONTEXT_Cpsr)]
msr APSR, r2

ldr sp, [r0, #(CONTEXT_Sp)]
ldr lr, [r0, #(CONTEXT_Lr)]
ldr pc, [r0, #(CONTEXT_Pc)]
ldr r2, [r0, #(CONTEXT_Pc)]
ldr sp, [r0, #(CONTEXT_Sp)]
bx r2

LOCAL_LABEL(No_Restore_CONTEXT_CONTROL):
ldr r2, [r0, #(CONTEXT_ContextFlags)]
Expand Down
17 changes: 12 additions & 5 deletions src/coreclr/pal/src/arch/i386/context2.S
Original file line number Diff line number Diff line change
Expand Up @@ -115,11 +115,12 @@ LOCAL_LABEL(Done_Restore_CONTEXT_FLOATING_POINT):
movdqu xmm7, [eax + CONTEXT_Xmm7]
LOCAL_LABEL(Done_Restore_CONTEXT_EXTENDED_REGISTERS):

// Restore Stack
mov esp, [eax + CONTEXT_Esp]

// Create a minimal frame
push DWORD PTR [eax + CONTEXT_Eip]
mov ebx, [eax + CONTEXT_Esp]
mov ecx, [eax + CONTEXT_Eip]
mov edx, [eax + CONTEXT_Eax]
mov [ebx - 4], ecx
mov [ebx - 8], edx

// Restore register(s)
mov ebp, [eax + CONTEXT_Ebp]
Expand All @@ -128,7 +129,13 @@ LOCAL_LABEL(Done_Restore_CONTEXT_EXTENDED_REGISTERS):
mov edx, [eax + CONTEXT_Edx]
mov ecx, [eax + CONTEXT_Ecx]
mov ebx, [eax + CONTEXT_Ebx]
mov eax, [eax + CONTEXT_Eax]

// Restore Stack
mov eax, [eax + CONTEXT_Esp]
sub eax, 8
mov esp, eax

pop eax

// Resume
ret
Expand Down
3 changes: 2 additions & 1 deletion src/coreclr/vm/amd64/Context.S
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,9 @@ NESTED_ENTRY ClrRestoreNonvolatileContextWorker, _TEXT, NoHandler
// exception handling, iret and ret can't be used because their shadow stack enforcement would not allow that transition,
// and using them would require writing to the shadow stack, which is not preferable. Instead, iret is partially
// simulated.
mov rax, [r10 + OFFSETOF__CONTEXT__Rip]
mov rsp, [r10 + OFFSETOF__CONTEXT__Rsp]
jmp qword ptr [r10 + OFFSETOF__CONTEXT__Rip]
jmp rax
Done_Restore_CONTEXT_CONTROL:

// The function was not asked to restore the control registers so we return back to the caller
Expand Down
3 changes: 2 additions & 1 deletion src/coreclr/vm/amd64/Context.asm
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,9 @@ NESTED_ENTRY ClrRestoreNonvolatileContextWorker, _TEXT
mov eax, [r10 + OFFSETOF__CONTEXT__EFlags]
push rax
popfq
mov rax, [r10 + OFFSETOF__CONTEXT__Rip]
mov rsp, [r10 + OFFSETOF__CONTEXT__Rsp]
jmp qword ptr [r10 + OFFSETOF__CONTEXT__Rip]
jmp rax
Done_Restore_CONTEXT_CONTROL:
; The function was not asked to restore the control registers so we return back to the caller
Expand Down

0 comments on commit 7aefd27

Please sign in to comment.