Skip to content
This repository was archived by the owner on Mar 24, 2022. It is now read-only.

Commit 18b4c42

Browse files
adjust signal handler to not always be so stack-hungry
Because the Lucet signal handler is run on any arbitrary thread that recives a signal, if the signal handler is installed anywhere in the process, it may be run on a signal stack we didn't set up. This means we are still bound to be careful about how much stack is present, which will often be SIGSTKSZ (observed to be ~8kb on Debian-likes, at least). `handle_signal` is the entrypoint for any signal in the process, so its call frame, locals, etc, are active for any path through the signal handler. For other paths through the signal handler that may involve subsequent calls, and in debug builds of lucet-runtime, the body of the `SignalBehavior::Default` arm would include several `UContext` local variables, as well as a `State` local which itself contains another `UContext`. x86_64 `UContext` are large, resulting in this match arm being responsible for ~5kb of stack usage in `handle_signal`, that is only _actually_ used in this match arm. So, by moving the arm to a closure and immediately calling it, we force Rust to defer that stack usage in debug builds to only be used when the closure is called. In release builds, the closure is likely inlined, but all intermediate copies of UContext _probably_ go away and the entire issue is _probably_ avoided. A subsequent commit will introduce tests around signal handler stack use for these various paths.
1 parent 2320b5b commit 18b4c42

1 file changed

Lines changed: 41 additions & 24 deletions

File tree

  • lucet-runtime/lucet-runtime-internals/src/instance

lucet-runtime/lucet-runtime-internals/src/instance/signals.rs

Lines changed: 41 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -180,31 +180,48 @@ extern "C" fn handle_signal(signum: c_int, siginfo_ptr: *mut siginfo_t, ucontext
180180
true
181181
}
182182
SignalBehavior::Default => {
183-
// safety: pointer is checked for null at the top of the function, and the
184-
// manpage guarantees that a siginfo_t will be passed as the second argument
185-
let siginfo = unsafe { *siginfo_ptr };
186-
let rip_addr = rip as usize;
187-
// If the trap table lookup returned unknown, it is a fatal error
188-
let unknown_fault = trapcode.is_none();
189-
190-
// If the trap was a segv or bus fault and the addressed memory was outside the
191-
// guard pages, it is also a fatal error
192-
let outside_guard = (siginfo.si_signo == SIGSEGV || siginfo.si_signo == SIGBUS)
193-
&& !inst.alloc.addr_in_guard_page(siginfo.si_addr_ext());
194-
195-
// record the fault and jump back to the host context
196-
inst.state = State::Faulted {
197-
details: FaultDetails {
198-
fatal: unknown_fault || outside_guard,
199-
trapcode: trapcode,
200-
rip_addr,
201-
// Details set to `None` here: have to wait until `verify_trap_safety` to
202-
// fill in these details, because access may not be signal safe.
203-
rip_addr_details: None,
204-
},
205-
siginfo,
206-
context: ctx.into(),
183+
/*
184+
* /!\ WARNING: LOAD-BEARING THUNK /!\
185+
*
186+
* This thunk, in debug builds, introduces multiple copies of UContext in the local
187+
* stack frame. This also includes a local `State`, which is quite large as well.
188+
* In total, this thunk accounts for roughly 5kb of stack use, where default signal
189+
* stack sizes are typically 8kb total.
190+
*
191+
* In code paths that do not pass through this (such as immediately reraising as a
192+
* host signal), the code in this thunk would force an exhaustion of more than half
193+
* the stack, significantly increasing the likelihood the Lucet signal handler may
194+
* overflow some other thread with a minimal stack size.
195+
*/
196+
let mut thunk = || {
197+
// safety: pointer is checked for null at the top of the function, and the
198+
// manpage guarantees that a siginfo_t will be passed as the second argument
199+
let siginfo = unsafe { *siginfo_ptr };
200+
let rip_addr = rip as usize;
201+
// If the trap table lookup returned unknown, it is a fatal error
202+
let unknown_fault = trapcode.is_none();
203+
204+
// If the trap was a segv or bus fault and the addressed memory was outside the
205+
// guard pages, it is also a fatal error
206+
let outside_guard = (siginfo.si_signo == SIGSEGV || siginfo.si_signo == SIGBUS)
207+
&& !inst.alloc.addr_in_guard_page(siginfo.si_addr_ext());
208+
209+
// record the fault and jump back to the host context
210+
inst.state = State::Faulted {
211+
details: FaultDetails {
212+
fatal: unknown_fault || outside_guard,
213+
trapcode: trapcode,
214+
rip_addr,
215+
// Details set to `None` here: have to wait until `verify_trap_safety` to
216+
// fill in these details, because access may not be signal safe.
217+
rip_addr_details: None,
218+
},
219+
siginfo,
220+
context: ctx.into(),
221+
};
207222
};
223+
224+
thunk();
208225
true
209226
}
210227
}

0 commit comments

Comments
 (0)