Skip to content

Commit 8bab6b4

Browse files
committed
Auto merge of #102460 - flba-eb:fix_85261_prevent_alloc_after_fork, r=thomcc
Prevent UB in child process after calling libc::fork After calling libc::fork, the child process tried to access a TLS variable when processing a panic. This caused a memory allocation which is UB in the child. To prevent this from happening, the panic handler will not access the TLS variable in case `panic::always_abort` was called before. Fixes #85261 (not only on Android systems, but also on Linux/QNX with TLS disabled, see issue for more details) Main drawbacks of this fix: * Panic messages can incorrectly omit `core::panic::PanicInfo` struct in case several panics (of multiple threads) occur at the same time. The handler cannot distinguish between multiple panics in different threads or recursive ones in the same thread, but the message will contain a hint about the uncertainty. * `panic_count::increase()` will be a bit slower as it has an additional `if`, but this should be irrelevant as it is only called in case of a panic.
2 parents 83438b4 + 0651abe commit 8bab6b4

File tree

1 file changed

+23
-4
lines changed

1 file changed

+23
-4
lines changed

std/src/panicking.rs

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -308,6 +308,14 @@ pub mod panic_count {
308308
// Additionally, the top bit of GLOBAL_PANIC_COUNT (GLOBAL_ALWAYS_ABORT_FLAG)
309309
// records whether panic::always_abort() has been called. This can only be
310310
// set, never cleared.
311+
// panic::always_abort() is usually called to prevent memory allocations done by
312+
// the panic handling in the child created by `libc::fork`.
313+
// Memory allocations performed in a child created with `libc::fork` are undefined
314+
// behavior in most operating systems.
315+
// Accessing LOCAL_PANIC_COUNT in a child created by `libc::fork` would lead to a memory
316+
// allocation. Only GLOBAL_PANIC_COUNT can be accessed in this situation. This is
317+
// sufficient because a child process will always have exactly one thread only.
318+
// See also #85261 for details.
311319
//
312320
// This could be viewed as a struct containing a single bit and an n-1-bit
313321
// value, but if we wrote it like that it would be more than a single word,
@@ -318,15 +326,26 @@ pub mod panic_count {
318326
// panicking thread consumes at least 2 bytes of address space.
319327
static GLOBAL_PANIC_COUNT: AtomicUsize = AtomicUsize::new(0);
320328

329+
// Return the state of the ALWAYS_ABORT_FLAG and number of panics.
330+
//
331+
// If ALWAYS_ABORT_FLAG is not set, the number is determined on a per-thread
332+
// base (stored in LOCAL_PANIC_COUNT), i.e. it is the amount of recursive calls
333+
// of the calling thread.
334+
// If ALWAYS_ABORT_FLAG is set, the number equals the *global* number of panic
335+
// calls. See above why LOCAL_PANIC_COUNT is not used.
321336
pub fn increase() -> (bool, usize) {
322-
(
323-
GLOBAL_PANIC_COUNT.fetch_add(1, Ordering::Relaxed) & ALWAYS_ABORT_FLAG != 0,
337+
let global_count = GLOBAL_PANIC_COUNT.fetch_add(1, Ordering::Relaxed);
338+
let must_abort = global_count & ALWAYS_ABORT_FLAG != 0;
339+
let panics = if must_abort {
340+
global_count & !ALWAYS_ABORT_FLAG
341+
} else {
324342
LOCAL_PANIC_COUNT.with(|c| {
325343
let next = c.get() + 1;
326344
c.set(next);
327345
next
328-
}),
329-
)
346+
})
347+
};
348+
(must_abort, panics)
330349
}
331350

332351
pub fn decrease() {

0 commit comments

Comments
 (0)