From b5c53ec0e00a5c510dd7e27a211e7d2370636477 Mon Sep 17 00:00:00 2001 From: Chris Pick Date: Mon, 13 Nov 2023 19:39:53 -0500 Subject: [PATCH 1/4] Restore handler in `test_old_sigaction_flags()` Multiple tests run within the same process in an indeterminate order. Restore the signal handler at the end of the test so other tests aren't affected. --- test/sys/test_signal.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/sys/test_signal.rs b/test/sys/test_signal.rs index ca25ff9ab0..bb8d5ea5cd 100644 --- a/test/sys/test_signal.rs +++ b/test/sys/test_signal.rs @@ -29,8 +29,10 @@ fn test_old_sigaction_flags() { ); let oact = unsafe { sigaction(SIGINT, &act) }.unwrap(); let _flags = oact.flags(); - let oact = unsafe { sigaction(SIGINT, &act) }.unwrap(); - let _flags = oact.flags(); + let _flags = unsafe { sigaction(SIGINT, &act) }.unwrap().flags(); + + // restore original + unsafe { sigaction(SIGINT, &oact) }.unwrap(); } #[test] From 30a6eee66c9b782c3edc711a11ff792b81df7c76 Mon Sep 17 00:00:00 2001 From: Chris Pick Date: Mon, 13 Nov 2023 19:47:08 -0500 Subject: [PATCH 2/4] Add `signal::sigaction_current()` Provide a way to query the currently installed sigaction. The decision to add `sigaction_current()` instead of just exposing the `sigaction_inner()` function was to avoid any confusion over the semantics of passing in a `None` `sigaction` argument (eg: someone thinking that it meant remove or reset the action). This builds towards #2172. --- src/sys/signal.rs | 32 +++++++++++++++++++++++++------ test/sys/test_signal.rs | 42 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 6 deletions(-) diff --git a/src/sys/signal.rs b/src/sys/signal.rs index 408471143f..3ea3ce258f 100644 --- a/src/sys/signal.rs +++ b/src/sys/signal.rs @@ -864,6 +864,16 @@ impl SigAction { } } +unsafe fn sigaction_inner(signal: Signal, sigaction: Option<&SigAction>) -> Result { + let mut oldact = mem::MaybeUninit::::uninit(); + + let res = libc::sigaction(signal as libc::c_int, + sigaction.map_or(ptr::null(), |sigaction| &sigaction.sigaction as *const libc::sigaction), + oldact.as_mut_ptr()); + + Errno::result(res).map(|_| SigAction { sigaction: oldact.assume_init() }) +} + /// Changes the action taken by a process on receipt of a specific signal. /// /// `signal` can be any signal except `SIGKILL` or `SIGSTOP`. On success, it returns the previous @@ -882,13 +892,23 @@ impl SigAction { /// pointer is valid. In that case, this function effectively dereferences a /// raw pointer of unknown provenance. pub unsafe fn sigaction(signal: Signal, sigaction: &SigAction) -> Result { - let mut oldact = mem::MaybeUninit::::uninit(); - - let res = libc::sigaction(signal as libc::c_int, - &sigaction.sigaction as *const libc::sigaction, - oldact.as_mut_ptr()); + sigaction_inner(signal, Some(sigaction)) +} - Errno::result(res).map(|_| SigAction { sigaction: oldact.assume_init() }) +/// Gets the current action a process will take on receipt of a specific signal. +/// +/// `signal` can be any signal except `SIGKILL` or `SIGSTOP`. On success, it returns the current +/// action for the given signal. The current action will always remain in place, unchanged. +/// +/// # Safety +/// +/// There is no guarantee that the old signal handler was installed +/// correctly. If it was installed by this crate, it will be. But if it was +/// installed by, for example, C code, then there is no guarantee its function +/// pointer is valid. In that case, this function effectively dereferences a +/// raw pointer of unknown provenance. +pub unsafe fn sigaction_current(signal: Signal) -> Result { + sigaction_inner(signal, None) } /// Signal management (see [signal(3p)](https://pubs.opengroup.org/onlinepubs/9699919799/functions/signal.html)) diff --git a/test/sys/test_signal.rs b/test/sys/test_signal.rs index bb8d5ea5cd..2d72ab6da3 100644 --- a/test/sys/test_signal.rs +++ b/test/sys/test_signal.rs @@ -35,6 +35,48 @@ fn test_old_sigaction_flags() { unsafe { sigaction(SIGINT, &oact) }.unwrap(); } +#[test] +fn test_current_sigaction() { + let _m = crate::SIGNAL_MTX.lock(); + + let oact = unsafe { + sigaction( + SIGINT, + &SigAction::new( + SigHandler::SigDfl, + SaFlags::empty(), + SigSet::empty(), + ), + ) + } + .unwrap(); + + assert_eq!( + unsafe { sigaction_current(SIGINT) }.unwrap().handler(), + SigHandler::SigDfl + ); + + unsafe { + sigaction( + SIGINT, + &SigAction::new( + SigHandler::SigIgn, + SaFlags::empty(), + SigSet::empty(), + ), + ) + } + .unwrap(); + + assert_eq!( + unsafe { sigaction_current(SIGINT) }.unwrap().handler(), + SigHandler::SigIgn + ); + + // restore original + unsafe { sigaction(SIGINT, &oact) }.unwrap(); +} + #[test] fn test_sigprocmask_noop() { sigprocmask(SigmaskHow::SIG_BLOCK, None, None) From 0e88e0e02c2c8f93d238cc63a9b3eff507c89b03 Mon Sep 17 00:00:00 2001 From: Chris Pick Date: Mon, 13 Nov 2023 20:06:50 -0500 Subject: [PATCH 3/4] Add `signal::sigaction_is_{default,ignore}()` Provide safe mechanisms to determine whether a signal's action is the default or ignore. This covers the majority of real-world calls to `sigaction()` with a NULL action without needing any unsafe code. Fixes #2172. --- src/sys/signal.rs | 16 ++++++++++++++++ test/sys/test_signal.rs | 4 ++++ 2 files changed, 20 insertions(+) diff --git a/src/sys/signal.rs b/src/sys/signal.rs index 3ea3ce258f..db372f0cc0 100644 --- a/src/sys/signal.rs +++ b/src/sys/signal.rs @@ -911,6 +911,22 @@ pub unsafe fn sigaction_current(signal: Signal) -> Result { sigaction_inner(signal, None) } +/// Whether the specified signal currently has its default action. +/// +/// `signal` can be any signal except `SIGKILL` or `SIGSTOP`. +pub fn sigaction_is_default(signal: Signal) -> Result { + // SAFETY: fetching the current action is safe if the handler isn't called + unsafe { sigaction_current(signal) }.map(|sigaction| sigaction.handler() == SigHandler::SigDfl) +} + +/// Whether the specified signal is currently ignored. +/// +/// `signal` can be any signal except `SIGKILL` or `SIGSTOP`. +pub fn sigaction_is_ignore(signal: Signal) -> Result { + // SAFETY: fetching the current action is safe if the handler isn't called + unsafe { sigaction_current(signal) }.map(|sigaction| sigaction.handler() == SigHandler::SigIgn) +} + /// Signal management (see [signal(3p)](https://pubs.opengroup.org/onlinepubs/9699919799/functions/signal.html)) /// /// Installs `handler` for the given `signal`, returning the previous signal diff --git a/test/sys/test_signal.rs b/test/sys/test_signal.rs index 2d72ab6da3..6b5694fc1e 100644 --- a/test/sys/test_signal.rs +++ b/test/sys/test_signal.rs @@ -55,6 +55,8 @@ fn test_current_sigaction() { unsafe { sigaction_current(SIGINT) }.unwrap().handler(), SigHandler::SigDfl ); + assert!(sigaction_is_default(SIGINT).unwrap()); + assert!(!sigaction_is_ignore(SIGINT).unwrap()); unsafe { sigaction( @@ -72,6 +74,8 @@ fn test_current_sigaction() { unsafe { sigaction_current(SIGINT) }.unwrap().handler(), SigHandler::SigIgn ); + assert!(!sigaction_is_default(SIGINT).unwrap()); + assert!(sigaction_is_ignore(SIGINT).unwrap()); // restore original unsafe { sigaction(SIGINT, &oact) }.unwrap(); From 907547144dcc23918d782295ae8f08f3e1a5e92d Mon Sep 17 00:00:00 2001 From: Chris Pick Date: Mon, 13 Nov 2023 20:21:29 -0500 Subject: [PATCH 4/4] Add changelog/2190.added.md --- changelog/2190.added.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog/2190.added.md diff --git a/changelog/2190.added.md b/changelog/2190.added.md new file mode 100644 index 0000000000..a17c10ae5b --- /dev/null +++ b/changelog/2190.added.md @@ -0,0 +1 @@ +Added `sigaction_is_default()`, `sigaction_is_ignore()`, and `sigaction_current()` functions \ No newline at end of file