Skip to content

Commit

Permalink
Abort on stack overflow instead of re-raising SIGSEGV
Browse files Browse the repository at this point in the history
We use guard pages that cause the process to abort to protect against
undefined behavior in the event of stack overflow.  We have a handler
that catches segfaults, prints out an error message if the segfault was
due to a stack overflow, then unregisters itself and returns to allow
the signal to be re-raised and kill the process.

This caused some confusion, as it was unexpected that safe code would be
able to cause a segfault, while it's easy to overflow the stack in safe
code.  To avoid this confusion, when we detect a segfault in the guard
page, abort instead of the previous behavior of re-raising the SIGSEGV.

To test this, we need to adapt the tests for segfault to actually check
the exit status.  Doing so revealed that the existing test for segfault
behavior was actually invalid; LLVM optimizes the explicit null pointer
reference down to an illegal instruction, so the program aborts with
SIGILL instead of SIGSEGV and the test didn't actually trigger the
signal handler at all.  Use a C helper function to get a null pointer
that LLVM can't optimize away, so we get our segfault instead.

This is a [breaking-change] if anyone is relying on the exact signal
raised to kill a process on stack overflow.

Closes #31273
  • Loading branch information
lambda committed Feb 6, 2016
1 parent 303892e commit ee79bfa
Show file tree
Hide file tree
Showing 4 changed files with 74 additions and 20 deletions.
38 changes: 21 additions & 17 deletions src/libstd/sys/unix/stack_overflow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,16 +83,19 @@ mod imp {
// were originally supposed to do.
//
// This handler currently exists purely to print an informative message
// whenever a thread overflows its stack. When run the handler always
// un-registers itself after running and then returns (to allow the original
// signal to be delivered again). By returning we're ensuring that segfaults
// do indeed look like segfaults.
// whenever a thread overflows its stack. We then abort to exit and
// indicate a crash, but to avoid a misleading SIGSEGV that might lead
// users to believe that unsafe code has accessed an invalid pointer; the
// SIGSEGV encountered when overflowing the stack is expected and
// well-defined.
//
// Returning from this kind of signal handler is technically not defined to
// work when reading the POSIX spec strictly, but in practice it turns out
// many large systems and all implementations allow returning from a signal
// handler to work. For a more detailed explanation see the comments on
// #26458.
// If this is not a stack overflow, the handler un-registers itself and
// then returns (to allow the original signal to be delivered again).
// Returning from this kind of signal handler is technically not defined
// to work when reading the POSIX spec strictly, but in practice it turns
// out many large systems and all implementations allow returning from a
// signal handler to work. For a more detailed explanation see the
// comments on #26458.
unsafe extern fn signal_handler(signum: libc::c_int,
info: *mut libc::siginfo_t,
_data: *mut libc::c_void) {
Expand All @@ -102,17 +105,18 @@ mod imp {
let addr = siginfo_si_addr(info);

// If the faulting address is within the guard page, then we print a
// message saying so.
// message saying so and abort.
if guard != 0 && guard - PAGE_SIZE <= addr && addr < guard {
report_overflow();
rtabort!("stack overflow");
} else {
// Unregister ourselves by reverting back to the default behavior.
let mut action: sigaction = mem::zeroed();
action.sa_sigaction = SIG_DFL;
sigaction(signum, &action, ptr::null_mut());

// See comment above for why this function returns.
}

// Unregister ourselves by reverting back to the default behavior.
let mut action: sigaction = mem::zeroed();
action.sa_sigaction = SIG_DFL;
sigaction(signum, &action, ptr::null_mut());

// See comment above for why this function returns.
}

static mut MAIN_ALTSTACK: *mut libc::c_void = ptr::null_mut();
Expand Down
5 changes: 5 additions & 0 deletions src/rt/rust_test_helpers.c
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,11 @@ rust_get_test_int() {
return 1;
}

char *
rust_get_null_ptr() {
return 0;
}

/* Debug helpers strictly to verify ABI conformance.
*
* FIXME (#2665): move these into a testcase when the testsuite
Expand Down
26 changes: 25 additions & 1 deletion src/test/run-pass/out-of-stack.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@
// ignore-musl

#![feature(asm)]
#![feature(libc)]

#[cfg(unix)]
extern crate libc;

use std::env;
use std::process::Command;
Expand All @@ -35,6 +39,24 @@ fn loud_recurse() {
black_box(()); // don't optimize this into a tail call. please.
}

#[cfg(unix)]
fn check_status(status: std::process::ExitStatus)
{
use libc;
use std::os::unix::process::ExitStatusExt;

assert!(!status.success());
assert!(status.signal() != Some(libc::SIGSEGV)
&& status.signal() != Some(libc::SIGBUS));
}

#[cfg(not(unix))]
fn check_status(status: std::process::ExitStatus)
{
assert!(!status.success());
}


fn main() {
let args: Vec<String> = env::args().collect();
if args.len() > 1 && args[1] == "silent" {
Expand Down Expand Up @@ -62,7 +84,9 @@ fn main() {
println!("testing: {}", mode);

let silent = Command::new(&args[0]).arg(mode).output().unwrap();
assert!(!silent.status.success());

check_status(silent.status);

let error = String::from_utf8_lossy(&silent.stderr);
assert!(error.contains("has overflowed its stack"),
"missing overflow message: {}", error);
Expand Down
25 changes: 23 additions & 2 deletions src/test/run-pass/segfault-no-out-of-stack.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,18 +15,39 @@ extern crate libc;
use std::process::{Command, ExitStatus};
use std::env;

#[link(name = "rust_test_helpers")]
extern {
fn rust_get_null_ptr() -> *mut ::libc::c_char;
}

#[cfg(unix)]
fn check_status(status: std::process::ExitStatus)
{
use libc;
use std::os::unix::process::ExitStatusExt;

assert!(status.signal() == Some(libc::SIGSEGV)
|| status.signal() == Some(libc::SIGBUS));
}

#[cfg(not(unix))]
fn check_status(status: std::process::ExitStatus)
{
assert!(!status.success());
}

fn main() {
let args: Vec<String> = env::args().collect();
if args.len() > 1 && args[1] == "segfault" {
unsafe { *(0 as *mut isize) = 1 }; // trigger a segfault
unsafe { *rust_get_null_ptr() = 1; }; // trigger a segfault
} else {
let segfault = Command::new(&args[0]).arg("segfault").output().unwrap();
let stderr = String::from_utf8_lossy(&segfault.stderr);
let stdout = String::from_utf8_lossy(&segfault.stdout);
println!("stdout: {}", stdout);
println!("stderr: {}", stderr);
println!("status: {}", segfault.status);
assert!(!segfault.status.success());
check_status(segfault.status);
assert!(!stderr.contains("has overflowed its stack"));
}
}

0 comments on commit ee79bfa

Please sign in to comment.