Skip to content

Guard against calling libc::exit multiple times on Linux. #126606

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

Merged
merged 12 commits into from
Jul 13, 2024
4 changes: 4 additions & 0 deletions library/std/src/rt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,10 @@ fn lang_start_internal(
rtabort!("drop of the panic payload panicked");
});
panic::catch_unwind(cleanup).map_err(rt_abort)?;
// Guard against multple threads calling `libc::exit` concurrently.
// See the documentation for `unique_thread_exit` for more information.
panic::catch_unwind(|| crate::sys::common::exit_guard::unique_thread_exit())
.map_err(rt_abort)?;
ret_code
}

Expand Down
59 changes: 59 additions & 0 deletions library/std/src/sys/pal/common/exit_guard.rs
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you move this file to sys, please? See #117276 for context.

Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
cfg_if::cfg_if! {
if #[cfg(target_os = "linux")] {
/// Mitigation for <https://github.com/rust-lang/rust/issues/126600>
///
/// On UNIX-like platforms (where `libc::exit` may not be thread-safe), ensure that only one
/// Rust thread calls `libc::exit` (or returns from `main`) by calling this function before
/// calling `libc::exit` (or returning from `main`).
///
/// Technically not enough to ensure soundness, since other code directly calling
/// libc::exit will still race with this.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// On UNIX-like platforms (where `libc::exit` may not be thread-safe), ensure that only one
/// Rust thread calls `libc::exit` (or returns from `main`) by calling this function before
/// calling `libc::exit` (or returning from `main`).
///
/// Technically not enough to ensure soundness, since other code directly calling
/// libc::exit will still race with this.
/// On glibc, `libc::exit` has been observed to not always be thread-safe.
/// It is currently unclear whether that is a glibc bug or allowed by the standard.
/// To mitigate this problem, we ensure that only one
/// Rust thread calls `libc::exit` (or returns from `main`) by calling this function before
/// calling `libc::exit` (or returning from `main`).
///
/// Technically, this is not enough to ensure soundness, since other code directly calling
/// `libc::exit` will still race with this.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd add the issue number #126600 as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue is linked above this on line 3, is that good?

///
/// *This function does not itself call `libc::exit`.* This is so it can also be used
/// to guard returning from `main`.
///
/// This function will return only the first time it is called in a process.
///
/// * If it is called again on the same thread as the first call, it will abort.
/// * If it is called again on a different thread, it will wait in a loop
/// (waiting for the process to exit).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add a note that we think this is a bug in glibc, and the POSIX and C standards expect exit to be thread-safe, but we add this defensively since we've seen segfaults in the wild?

pub(crate) fn unique_thread_exit() {
let this_thread_id = unsafe { libc::pthread_self() };
use crate::sync::{Mutex, PoisonError};
static EXITING_THREAD_ID: Mutex<Option<libc::pthread_t>> = Mutex::new(None);
let mut exiting_thread_id =
EXITING_THREAD_ID.lock().unwrap_or_else(PoisonError::into_inner);
match *exiting_thread_id {
None => {
// This is the first thread to call `unique_thread_exit`,
// and this is the first time it is called.
// Set EXITING_THREAD_ID to this thread's ID and return.
*exiting_thread_id = Some(this_thread_id);
},
Some(exiting_thread_id) if exiting_thread_id == this_thread_id => {
// This is the first thread to call `unique_thread_exit`,
// but this is the second time it is called.
// Abort the process.
core::panicking::panic_nounwind("std::process::exit called re-entrantly")
}
Some(_) => {
// This is not the first thread to call `unique_thread_exit`.
// Pause until the process exits.
drop(exiting_thread_id);
loop {
// Safety: libc::pause is safe to call.
unsafe { libc::pause(); }
}
}
}
}
} else {
/// Mitigation for <https://github.com/rust-lang/rust/issues/126600>
///
/// Mitigation is ***NOT*** implemented on this platform, either because this platform
/// is not affected, or because mitigation is not yet implemented for this platform.
pub(crate) fn unique_thread_exit() {
// Mitigation not required on platforms where `exit` is thread-safe.
}
}
}
1 change: 1 addition & 0 deletions library/std/src/sys/pal/common/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#![allow(dead_code)]

pub mod alloc;
pub mod exit_guard;
pub mod small_c_string;

#[cfg(test)]
Expand Down
1 change: 1 addition & 0 deletions library/std/src/sys/pal/unix/os.rs
Original file line number Diff line number Diff line change
Expand Up @@ -758,6 +758,7 @@ pub fn home_dir() -> Option<PathBuf> {
}

pub fn exit(code: i32) -> ! {
crate::sys::common::exit_guard::unique_thread_exit();
unsafe { libc::exit(code as c_int) }
}

Expand Down
Loading