LadybirdBrowser/ladybird
1
1
Fork 0
mirror of https://github.com/LadybirdBrowser/ladybird.git synced 2024-09-19 09:17:38 +03:00
Code Issues Packages Projects Releases Wiki Activity

Kernel: Properly align stack for signal handlers

Browse Source 
The System V ABI requires that the stack is 16-byte aligned on function
call. Confusingly, however, they mean that the stack must be aligned
this way **before** the `CALL` instruction is executed. That instruction
pushes the return value onto the stack, so the callee will actually see
the stack pointer as a value `sizeof(FlatPtr)` smaller.

The signal trampoline was written with this in mind, but `setup_stack`
aligned the entire stack, *including the return address* to a 16-byte
boundary. Because of this, the trampoline subtracted too much from the
stack pointer, thus misaligning it.

This was not a problem on i686 because we didn't execute any
instructions from signal handlers that would require memory operands to
be aligned to more than 4 bytes. This is not the case, however, on
x86_64, where SSE instructions are enabled by default and they require
16-byte aligned operands. Running such instructions raised a GP fault,
immediately killing the offending program with a SIGSEGV signal.

This issue caused TestKernelAlarm to fail in LibC when ran locally, and
at one point, the zsh port was affected too.

Fixes #9291
This commit is contained in:
Daniel Bertalan 2021-10-24 17:34:59 +02:00 committed by Andreas Kling
parent b138b4c83f
commit db71c36657
Notes: sideshowbarker 2024-07-18 01:56:19 +09:00 
Author: https://github.com/BertalanD
Commit: https://github.com/SerenityOS/serenity/commit/db71c36657d
Pull-request: https://github.com/SerenityOS/serenity/pull/10625
Issue: https://github.com/SerenityOS/serenity/issues/9291
1 changed files with 12 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

23
Kernel/Thread.cpp
View File

@ -932,11 +932,11 @@ DispatchSignalResult Thread::dispatch_signal(u8 signal)
#if ARCH(I386)
// Align the stack to 16 bytes.
// Note that we push 56 bytes (4 * 14) on to the stack,
// so we need to account for this here.
// 56 % 16 = 8, so we only need to take 8 bytes into consideration for
// Note that we push 52 bytes (4 * 13) on to the stack
// before the return address, so we need to account for this here.
// 56 % 16 = 4, so we only need to take 4 bytes into consideration for
// the stack alignment.
FlatPtr stack_alignment = (stack - 8) % 16;
FlatPtr stack_alignment = (stack - 4) % 16;
stack -= stack_alignment;
push_value_on_user_stack(stack, ret_flags);
@ -952,12 +952,12 @@ DispatchSignalResult Thread::dispatch_signal(u8 signal)
push_value_on_user_stack(stack, state.edi);
#else
// Align the stack to 16 bytes.
// Note that we push 176 bytes (8 * 22) on to the stack,
// so we need to account for this here.
// 22 % 2 = 0, so we dont need to take anything into consideration
// for the alignment.
// Note that we push 168 bytes (8 * 21) on to the stack
// before the return address, so we need to account for this here.
// 168 % 16 = 8, so we only need to take 8 bytes into consideration for
// the stack alignment.
// We also are not allowed to touch the thread's red-zone of 128 bytes
FlatPtr stack_alignment = stack % 16;
FlatPtr stack_alignment = (stack - 8) % 16;
stack -= 128 + stack_alignment;
push_value_on_user_stack(stack, ret_flags);
@ -986,13 +986,14 @@ DispatchSignalResult Thread::dispatch_signal(u8 signal)
push_value_on_user_stack(stack, signal);
push_value_on_user_stack(stack, handler_vaddr.get());
VERIFY((stack % 16) == 0);
push_value_on_user_stack(stack, 0); // push fake return address
// We write back the adjusted stack value into the register state.
// We have to do this because we can't just pass around a reference to a packed field, as it's UB.
state.set_userspace_sp(stack);
VERIFY((stack % 16) == 0);
};
// We now place the thread state on the userspace stack.