Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sigaltstack() and SA_ONSTACK are stubbed #809

Closed
nyh opened this issue Nov 3, 2016 · 3 comments
Closed

sigaltstack() and SA_ONSTACK are stubbed #809

nyh opened this issue Nov 3, 2016 · 3 comments

Comments

@nyh
Copy link
Contributor

nyh commented Nov 3, 2016

Our sigaltstack() is a stub, pretending to have succeeded but does nothing. Accordingly, sigaction()'s SA_ONSTACK flag is also ignored.

This stub is usually good enough, because OSv's signal handling currently differs greatly from Linux, and when an application sends a signal to itself with kill() or alarm(), the signal handler is run in a new thread and not the original thread - so a separate stack is not necessary (or relevant).

The exception is when OSv uses generate_signal() to send a signal to the current thread - and this currently happens only for SIGFPE (divide by zero) and SIGSEGV. In those cases, the application may really need the separate stack for the signal handler, either because the application stack is very small (e.g., this is the case in golang, #522) or because the application stack already ran out (notably, we may have gotten this SIGSEGV because we already overflowed the stack!).

@nyh
Copy link
Contributor Author

nyh commented Feb 9, 2017

I think fixing this is more straightforward than fixing the similar (but harder) #808.

Luckily, OSv already uses the hardware "IST" (interrupt stack table) to switch to a different stack (a per-thread exception stack) when getting an exception (such as a page fault). So our page_fault() code already runs on a safe stack - not the user's stack. If the page fault code decides to generate a SIGSEGV signal, it calls vm_sigsegv(exception_frame *ef) , which in turn calls osv::handle_mmap_fault(ef), which in turn calls generate_signal(ef), which in turn calls arch::build_signal_frame(ef). That last function manipulates the stack in ef->rsp (skips the red zone, etc.) to ensure that the signal handler runs when the exception handler returns. However, if we have a sigaltstack(), we should switch to that stack instead, and run the handler on it (and switch back to the original stack when the handler returns).

@benoit-canet
Copy link
Contributor

@nyh: out of curiosity how rip is modified by build_signal_frame ?

@nyh nyh closed this as completed in bf7fbc3 Feb 12, 2017
@nyh
Copy link
Contributor Author

nyh commented Feb 12, 2017

@benoit-canet build_signal_frame is called while we're handling an x86 exception (e.g., page fault). When this happens, the exception entry code (in entry.S) saves the pre-exception state (including rip and stack pointer) in an "exception frame" structure. It also allows the handler to set a new rip and rsp which will be loaded when the exception handler finishes. So build_signal_frame() sets these two fields, and when it returns, these two fields are used to "restore" rip and rsp to new values, so the processor jumps to the handler code instead of returning to the original code. When the handler code returns (although, normally it doesn't...), we can restore the original rip/rsp that are still saved in the exception frame structure copy on the handler's stack.

myechuri pushed a commit to myechuri/osv that referenced this issue Jun 22, 2017
When an application gets a SIGSEGV because it overflowed the thread's
stack, it is pointless to try to run a signal handler on the same stack
which has no room to run on. This is why Linux provides a sigaltstack()
function which allows the user to specify a (per-thread) stack for running
signal handlers, and a SA_ONSTACK flag for sigaction() to ask to use the
alternative stack for a specific handler.

So far these features were stubbed, and this patch provides a complete
implementation, and also a test - which overflows the stack and tries
to recover with a signal handler (without SA_ONSTACK properly working,
the test crashes when it tries to run the signal handler).

Fixes cloudius-systems#809.

Proper support of sigaltstack() is important for golang (see issue cloudius-systems#522).

Signed-off-by: Nadav Har'El <[email protected]>
Message-Id: <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants