Browse Source

Fix reference getting moved

The sigaction handler called map on an option, creating a pointer to a
move value. This in turned caused UB for signal handlers. Avoid using
pointers directly, and instead prefer references.
Xavier L'Heureux 5 years ago
parent
commit
0a558de76c
5 changed files with 29 additions and 32 deletions
  1. 13 17
      src/header/signal/mod.rs
  2. 10 9
      src/platform/linux/signal.rs
  3. 1 1
      src/platform/pal/signal.rs
  4. 4 2
      src/platform/redox/signal.rs
  5. 1 3
      tests/signal.c

+ 13 - 17
src/header/signal/mod.rs

@@ -1,6 +1,6 @@
 //! signal implementation for Redox, following http://pubs.opengroup.org/onlinepubs/7908799/xsh/signal.h.html
 
-use core::{mem, ptr};
+use core::mem;
 
 use cbitset::BitSet;
 
@@ -21,9 +21,9 @@ pub mod sys;
 
 type SigSet = BitSet<[c_ulong; 1]>;
 
-pub (crate) const SIG_DFL: usize = 0;
-pub (crate) const SIG_IGN: usize = 1;
-pub (crate) const SIG_ERR: isize = -1;
+pub(crate) const SIG_DFL: usize = 0;
+pub(crate) const SIG_IGN: usize = 1;
+pub(crate) const SIG_ERR: isize = -1;
 
 pub const SIG_BLOCK: c_int = 0;
 pub const SIG_UNBLOCK: c_int = 1;
@@ -86,18 +86,12 @@ pub unsafe extern "C" fn sigaction(
     act: *const sigaction,
     oact: *mut sigaction,
 ) -> c_int {
-    let act_opt = if !act.is_null() {
-        let mut act_clone = (*act).clone();
+    let act_opt = act.as_ref().map(|act| {
+        let mut act_clone = act.clone();
         act_clone.sa_flags |= SA_RESTORER as c_ulong;
-        Some(act_clone)
-    } else {
-        None
-    };
-    println!("sig => {:?}", act_opt);
-    println!("osig => {:?}", (*oact));
-    let out = Sys::sigaction(sig, act_opt.map_or(ptr::null_mut(), |x| &x), oact);
-    println!("aosig => {:?}", (*oact).sa_handler);
-    out
+        act_clone
+    });
+    Sys::sigaction(sig, act_opt.as_ref(), oact.as_mut())
 }
 
 #[no_mangle]
@@ -198,8 +192,10 @@ extern "C" {
 }
 
 #[no_mangle]
-pub extern "C" fn signal(sig: c_int, func: Option<extern "C" fn(c_int)>) -> Option<extern "C" fn(c_int)> {
-    println!("{:?}", func);
+pub extern "C" fn signal(
+    sig: c_int,
+    func: Option<extern "C" fn(c_int)>,
+) -> Option<extern "C" fn(c_int)> {
     let sa = sigaction {
         sa_handler: func,
         sa_flags: SA_RESTART as c_ulong,

+ 10 - 9
src/platform/linux/signal.rs

@@ -35,15 +35,16 @@ impl PalSignal for Sys {
         e(unsafe { syscall!(SETITIMER, which, new, old) }) as c_int
     }
 
-    unsafe fn sigaction(sig: c_int, act: *const sigaction, oact: *mut sigaction) -> c_int {
-        println!("in: {:?}", (*act));
-        e(syscall!(
-            RT_SIGACTION,
-            sig,
-            act,
-            oact,
-            mem::size_of::<sigset_t>()
-        )) as c_int
+    fn sigaction(sig: c_int, act: Option<&sigaction>, oact: Option<&mut sigaction>) -> c_int {
+        e(unsafe {
+            syscall!(
+                RT_SIGACTION,
+                sig,
+                act.map_or_else(core::ptr::null, |x| x as *const _),
+                oact.map_or_else(core::ptr::null_mut, |x| x as *mut _),
+                mem::size_of::<sigset_t>()
+            )
+        }) as c_int
     }
 
     fn sigaltstack(ss: *const stack_t, old_ss: *mut stack_t) -> c_int {

+ 1 - 1
src/platform/pal/signal.rs

@@ -15,7 +15,7 @@ pub trait PalSignal: Pal {
 
     fn setitimer(which: c_int, new: *const itimerval, old: *mut itimerval) -> c_int;
 
-    unsafe fn sigaction(sig: c_int, act: *const sigaction, oact: *mut sigaction) -> c_int;
+    fn sigaction(sig: c_int, act: Option<&sigaction>, oact: Option<&mut sigaction>) -> c_int;
 
     fn sigaltstack(ss: *const stack_t, old_ss: *mut stack_t) -> c_int;
 

+ 4 - 2
src/platform/redox/signal.rs

@@ -111,7 +111,9 @@ impl PalSignal for Sys {
         } else {
             let m = (*act).sa_mask;
             let sa_handler = mem::transmute((*act).sa_handler);
-    println!("signal called with {:x}", unsafe { mem::transmute::<_, usize>(sa_handler) });
+            println!("signal called with {:x}", unsafe {
+                mem::transmute::<_, usize>(sa_handler)
+            });
             Some(syscall::SigAction {
                 sa_handler,
                 sa_mask: [m as u64, 0],
@@ -138,7 +140,7 @@ impl PalSignal for Sys {
             (*oact).sa_mask = m[0] as c_ulong;
             (*oact).sa_flags = old.sa_flags.bits() as c_ulong;
         }
-        println!("after : {:?}", oact);
+        println!("after : {:?}", (*oact));
         ret
     }
 

+ 1 - 3
tests/signal.c

@@ -11,12 +11,10 @@ void handler(int sig) {
 }
 
 int main(void) {
-    printf("handler: %x\n", handler);
     void (*signal_status)(int) = signal(SIGUSR1, handler);
     ERROR_IF(signal, signal_status, == SIG_ERR);
     signal_status = signal(SIGUSR1, handler);
-    ERROR_IF(signal, signal_status, == SIG_ERR);
-    printf("out: %x\n", signal_status);
+    ERROR_IF(signal, signal_status, != handler);
 
     puts("Raising...");