dnrops
/
rust
mirror of https://github.com/rust-lang/rust.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Rollup merge of #127843 - workingjubilee:break-up-big-ass-stack-overflow-fn, r=joboet 

unix: document unsafety for std `sig{action,altstack}`

I found many surprising elements here while trying to wrap a measly 5 functions with `unsafe`. I would rather not "just" mindlessly wrap this code with `unsafe { }`, so I decided to document it properly.

On Unix, this code covers the "create and setup signal handler" part of the stack overflow code, and serves as the primary safety boundary for the signal handler. It is rarely audited, very gnarly, and worth extra attention. It calls other unsafe functions defined in this module, but "can we correctly map the right memory, or find the right address ranges?" are separate questions, and get increasingly platform-specific. The question here is the more general "are we doing everything in the correct order, and setting up the handler in the correct way?"

As part of this audit, I noticed that we do some peculiar things that we should probably refrain from. However, I avoided making changes that I deemed might have a different final result in Rust programs. I did, however, reorder some events so that the signal handler is installed _after_ we install the alternate stack. We do not run much code between these events, but it is probably best if the timespan between the handler being available and the new stack being installed is 0 nanoseconds.
This commit is contained in:
Matthias Krüger 2024-07-20 13:24:53 +02:00 committed by GitHub
parent 9a6f8ccf3a 529fcbcd6d
commit 2ee3668ad5
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
1 changed files with 61 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

85
library/std/src/sys/pal/unix/stack_overflow.rs
View File

@ -87,13 +87,18 @@ mod imp {
// 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.
/// SIGSEGV/SIGBUS entry point
/// # Safety
/// Rust doesn't call this, it *gets called*.
#[forbid(unsafe_op_in_unsafe_fn)]
unsafe extern "C" fn signal_handler(
signum: libc::c_int,
info: *mut libc::siginfo_t,
_data: *mut libc::c_void,
) {
let (start, end) = GUARD.get();
let addr = (*info).si_addr() as usize;
// SAFETY: this pointer is provided by the system and will always point to a valid `siginfo_t`.
let addr = unsafe { (*info).si_addr().addr() };
// If the faulting address is within the guard page, then we print a
// message saying so and abort.
@ -105,9 +110,11 @@ mod imp {
rtabort!("stack overflow");
} else {
// Unregister ourselves by reverting back to the default behavior.
let mut action: sigaction = mem::zeroed();
// SAFETY: assuming all platforms define struct sigaction as "zero-initializable"
let mut action: sigaction = unsafe { mem::zeroed() };
action.sa_sigaction = SIG_DFL;
sigaction(signum, &action, ptr::null_mut());
// SAFETY: pray this is a well-behaved POSIX implementation of fn sigaction
unsafe { sigaction(signum, &action, ptr::null_mut()) };
// See comment above for why this function returns.
}
@ -117,32 +124,45 @@ mod imp {
static MAIN_ALTSTACK: AtomicPtr<libc::c_void> = AtomicPtr::new(ptr::null_mut());
static NEED_ALTSTACK: AtomicBool = AtomicBool::new(false);
/// # Safety
/// Must be called only once
#[forbid(unsafe_op_in_unsafe_fn)]
pub unsafe fn init() {
PAGE_SIZE.store(os::page_size(), Ordering::Relaxed);
// Always write to GUARD to ensure the TLS variable is allocated.
let guard = install_main_guard().unwrap_or(0..0);
let guard = unsafe { install_main_guard().unwrap_or(0..0) };
GUARD.set((guard.start, guard.end));
let mut action: sigaction = mem::zeroed();
// SAFETY: assuming all platforms define struct sigaction as "zero-initializable"
let mut action: sigaction = unsafe { mem::zeroed() };
for &signal in &[SIGSEGV, SIGBUS] {
sigaction(signal, ptr::null_mut(), &mut action);
// SAFETY: just fetches the current signal handler into action
unsafe { sigaction(signal, ptr::null_mut(), &mut action) };
// Configure our signal handler if one is not already set.
if action.sa_sigaction == SIG_DFL {
if !NEED_ALTSTACK.load(Ordering::Relaxed) {
// haven't set up our sigaltstack yet
NEED_ALTSTACK.store(true, Ordering::Release);
let handler = unsafe { make_handler(true) };
MAIN_ALTSTACK.store(handler.data, Ordering::Relaxed);
mem::forget(handler);
}
action.sa_flags = SA_SIGINFO | SA_ONSTACK;
action.sa_sigaction = signal_handler as sighandler_t;
sigaction(signal, &action, ptr::null_mut());
NEED_ALTSTACK.store(true, Ordering::Relaxed);
// SAFETY: only overriding signals if the default is set
unsafe { sigaction(signal, &action, ptr::null_mut()) };
}
}
let handler = make_handler(true);
MAIN_ALTSTACK.store(handler.data, Ordering::Relaxed);
mem::forget(handler);
}
/// # Safety
/// Must be called only once
#[forbid(unsafe_op_in_unsafe_fn)]
pub unsafe fn cleanup() {
drop_handler(MAIN_ALTSTACK.load(Ordering::Relaxed));
// FIXME: I probably cause more bugs than I'm worth!
// see https://github.com/rust-lang/rust/issues/111272
unsafe { drop_handler(MAIN_ALTSTACK.load(Ordering::Relaxed)) };
}
unsafe fn get_stack() -> libc::stack_t {
@ -187,34 +207,48 @@ mod imp {
libc::stack_t { ss_sp: stackp, ss_flags: 0, ss_size: sigstack_size }
}
/// # Safety
/// Mutates the alternate signal stack
#[forbid(unsafe_op_in_unsafe_fn)]
pub unsafe fn make_handler(main_thread: bool) -> Handler {
if !NEED_ALTSTACK.load(Ordering::Relaxed) {
if !NEED_ALTSTACK.load(Ordering::Acquire) {
return Handler::null();
}
if !main_thread {
// Always write to GUARD to ensure the TLS variable is allocated.
let guard = current_guard().unwrap_or(0..0);
let guard = unsafe { current_guard() }.unwrap_or(0..0);
GUARD.set((guard.start, guard.end));
}
let mut stack = mem::zeroed();
sigaltstack(ptr::null(), &mut stack);
// SAFETY: assuming stack_t is zero-initializable
let mut stack = unsafe { mem::zeroed() };
// SAFETY: reads current stack_t into stack
unsafe { sigaltstack(ptr::null(), &mut stack) };
// Configure alternate signal stack, if one is not already set.
if stack.ss_flags & SS_DISABLE != 0 {
stack = get_stack();
sigaltstack(&stack, ptr::null_mut());
// SAFETY: We warned our caller this would happen!
unsafe {
stack = get_stack();
sigaltstack(&stack, ptr::null_mut());
}
Handler { data: stack.ss_sp as *mut libc::c_void }
} else {
Handler::null()
}
}
/// # Safety
/// Must be called
/// - only with our handler or nullptr
/// - only when done with our altstack
/// This disables the alternate signal stack!
#[forbid(unsafe_op_in_unsafe_fn)]
pub unsafe fn drop_handler(data: *mut libc::c_void) {
if !data.is_null() {
let sigstack_size = sigstack_size();
let page_size = PAGE_SIZE.load(Ordering::Relaxed);
let stack = libc::stack_t {
let disabling_stack = libc::stack_t {
ss_sp: ptr::null_mut(),
ss_flags: SS_DISABLE,
// Workaround for bug in macOS implementation of sigaltstack
@ -223,10 +257,11 @@ mod imp {
// both ss_sp and ss_size should be ignored in this case.
ss_size: sigstack_size,
};
sigaltstack(&stack, ptr::null_mut());
// We know from `get_stackp` that the alternate stack we installed is part of a mapping
// that started one page earlier, so walk back a page and unmap from there.
munmap(data.sub(page_size), sigstack_size + page_size);
// SAFETY: we warned the caller this disables the alternate signal stack!
unsafe { sigaltstack(&disabling_stack, ptr::null_mut()) };
// SAFETY: We know from `get_stackp` that the alternate stack we installed is part of
// a mapping that started one page earlier, so walk back a page and unmap from there.
unsafe { munmap(data.sub(page_size), sigstack_size + page_size) };
}
}
@ -455,6 +490,7 @@ mod imp {
}
#[cfg(any(target_os = "macos", target_os = "openbsd", target_os = "solaris"))]
// FIXME: I am probably not unsafe.
unsafe fn current_guard() -> Option<Range<usize>> {
let stackptr = get_stack_start()?;
let stackaddr = stackptr.addr();
@ -469,6 +505,7 @@ mod imp {
target_os = "netbsd",
target_os = "l4re"
))]
// FIXME: I am probably not unsafe.
unsafe fn current_guard() -> Option<Range<usize>> {
let mut ret = None;
let mut attr: libc::pthread_attr_t = crate::mem::zeroed();