sys_util: factor out signal manipulation from signalfd into signal

Move creating sigsets and blocking/unblocking signals form signalfd
module to signal module so they are usable by other parties as well.

BUG=chromium:800626
TEST=cargo test --features=plugin

Change-Id: I281ce784ed6cb341cc1e7cf2784f6fb1e8cc894d
Signed-off-by: Dmitry Torokhov <dtor@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/930461
Reviewed-by: Zach Reizner <zachr@chromium.org>

5 files changed, 130 insertions, 85 deletions

diff --git a/src/linux.rs b/src/linux.rs
index b4c76d9..c29819e 100644
--- a/src/linux.rs
+++ b/src/linux.rs
@@ -536,7 +536,7 @@ fn setup_vcpu(kvm: &Kvm,
             unsafe {
                 extern "C" fn handle_signal() {}
                 // Our signal handler does nothing and is trivially async signal safe.
-                register_signal_handler(0, handle_signal)
+                register_signal_handler(SIGRTMIN() + 0, handle_signal)
                     .expect("failed to register vcpu signal handler");
             }
 
@@ -696,7 +696,7 @@ fn run_control(vm: &mut Vm,
     // re-enter the VM.
     kill_signaled.store(true, Ordering::SeqCst);
     for handle in vcpu_handles {
-        match handle.kill(0) {
+        match handle.kill(SIGRTMIN() + 0) {
             Ok(_) => {
                 if let Err(e) = handle.join() {
                     error!("failed to join vcpu thread: {:?}", e);
diff --git a/src/plugin/mod.rs b/src/plugin/mod.rs
index c5e8766..44cba39 100644
--- a/src/plugin/mod.rs
+++ b/src/plugin/mod.rs
@@ -25,7 +25,8 @@ use protobuf::ProtobufError;
 use io_jail::{self, Minijail};
 use kvm::{Kvm, Vm, Vcpu, VcpuExit, IoeventAddress, NoDatamatch};
 use sys_util::{EventFd, MmapError, Killable, SignalFd, SignalFdError, Poller, Pollable,
-               GuestMemory, Result as SysResult, Error as SysError, register_signal_handler,
+               GuestMemory, Result as SysResult, Error as SysError,
+               register_signal_handler, SIGRTMIN,
                geteuid, getegid};
 
 use Config;
@@ -306,7 +307,7 @@ pub fn run_vcpus(kvm: &Kvm,
             unsafe {
                 extern "C" fn handle_signal() {}
                 // Our signal handler does nothing and is trivially async signal safe.
-                register_signal_handler(0, handle_signal)
+                register_signal_handler(SIGRTMIN() + 0, handle_signal)
                     .expect("failed to register vcpu signal handler");
             }
 
@@ -560,7 +561,7 @@ pub fn run_config(cfg: Config) -> Result<()> {
     // blocked connections.
     plugin.signal_kill().map_err(Error::PluginKill)?;
     for handle in vcpu_handles {
-        match handle.kill(0) {
+        match handle.kill(SIGRTMIN() + 0) {
             Ok(_) => {
                 if let Err(e) = handle.join() {
                     error!("failed to join vcpu thread: {:?}", e);
diff --git a/src/plugin/process.rs b/src/plugin/process.rs
index 99bdc11..aa4a907 100644
--- a/src/plugin/process.rs
+++ b/src/plugin/process.rs
@@ -374,7 +374,7 @@ impl Process {
                 .enumerate() {
             if cpu_mask & (1 << cpu_id) != 0 {
                 per_cpu_state.lock().unwrap().request_pause(user_data);
-                if let Err(e) = handle.kill(0) {
+                if let Err(e) = handle.kill(SIGRTMIN() + 0) {
                     error!("failed to interrupt vcpu {}: {:?}", cpu_id, e);
                 }
             }
diff --git a/sys_util/src/signal.rs b/sys_util/src/signal.rs
index 0fc4612..2777613 100644
--- a/sys_util/src/signal.rs
+++ b/sys_util/src/signal.rs
@@ -2,12 +2,34 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-use libc::{c_int, pthread_t, signal, pthread_kill, SIG_ERR, EINVAL};
+use libc::{c_int, signal,
+           sigset_t, sigaddset, sigemptyset, sigismember,
+           pthread_t, pthread_kill, pthread_sigmask,
+           SIG_BLOCK, SIG_UNBLOCK, SIG_ERR, EINVAL};
 
+use std::mem;
+use std::ptr::null_mut;
+use std::result;
 use std::thread::JoinHandle;
 use std::os::unix::thread::JoinHandleExt;
 
-use {Error, Result, errno_result};
+use {errno, errno_result};
+
+#[derive(Debug)]
+pub enum Error {
+    /// Couldn't create a sigset.
+    CreateSigset(errno::Error),
+    /// The wrapped signal has already been blocked.
+    SignalAlreadyBlocked(c_int),
+    /// Failed to check if the requested signal is in the blocked set already.
+    CompareBlockedSignals(errno::Error),
+    /// The signal could not be blocked.
+    BlockSignal(errno::Error),
+    /// The signal could not be unblocked.
+    UnblockSignal(errno::Error),
+}
+
+pub type SignalResult<T> = result::Result<T, Error>;
 
 #[link(name = "c")]
 extern "C" {
@@ -27,21 +49,24 @@ pub fn SIGRTMAX() -> c_int {
     unsafe { __libc_current_sigrtmax() }
 }
 
-fn valid_signal_num(num: u8) -> bool {
-    (num as c_int) + SIGRTMIN() <= SIGRTMAX()
+fn valid_signal_num(num: c_int) -> bool {
+    num >= SIGRTMIN() && num <= SIGRTMAX()
 }
 
-/// Registers `handler` as the signal handler of signum `num + SIGRTMIN`.
+/// Registers `handler` as the signal handler of signum `num`.
 ///
-/// The value of `num + SIGRTMIN` must not exceed `SIGRTMAX`.
+/// The value of `num` must be within [`SIGRTMIN`, `SIGRTMAX`] range.
 ///
 /// This is considered unsafe because the given handler will be called asynchronously, interrupting
 /// whatever the thread was doing and therefore must only do async-signal-safe operations.
-pub unsafe fn register_signal_handler(num: u8, handler: extern "C" fn() -> ()) -> Result<()> {
+pub unsafe fn register_signal_handler(
+    num: c_int,
+    handler: extern "C" fn() -> (),
+) -> errno::Result<()> {
     if !valid_signal_num(num) {
-        return Err(Error::new(EINVAL));
+        return Err(errno::Error::new(EINVAL));
     }
-    let ret = signal((num as i32) + SIGRTMIN(), handler as *const () as usize);
+    let ret = signal(num, handler as *const () as usize);
     if ret == SIG_ERR {
         return errno_result();
     }
@@ -49,6 +74,66 @@ pub unsafe fn register_signal_handler(num: u8, handler: extern "C" fn() -> ()) -
     Ok(())
 }
 
+/// Creates `sigset` from an array of signal numbers.
+///
+/// This is a helper function used when we want to manipulate signals.
+pub fn create_sigset(signals: &[c_int]) -> errno::Result<sigset_t> {
+    // sigset will actually be initialized by sigemptyset below.
+    let mut sigset: sigset_t = unsafe { mem::zeroed() };
+
+    // Safe - return value is checked.
+    let ret = unsafe { sigemptyset(&mut sigset) };
+    if ret < 0 {
+        return errno_result();
+    }
+
+    for signal in signals {
+        // Safe - return value is checked.
+        let ret = unsafe { sigaddset(&mut sigset, *signal) };
+        if ret < 0 {
+            return errno_result();
+        }
+    }
+
+    Ok(sigset)
+}
+
+/// Masks given signal.
+///
+/// If signal is already blocked the call will fail with Error::SignalAlreadyBlocked
+/// result.
+pub fn block_signal(num: c_int) -> SignalResult<()> {
+    let sigset = create_sigset(&[num]).map_err(Error::CreateSigset)?;
+
+    // Safe - return values are checked.
+    unsafe {
+        let mut old_sigset: sigset_t = mem::zeroed();
+        let ret = pthread_sigmask(SIG_BLOCK, &sigset, &mut old_sigset as *mut sigset_t);
+        if ret < 0 {
+            return Err(Error::BlockSignal(errno::Error::last()));
+        }
+        let ret = sigismember(&old_sigset, num);
+        if ret < 0 {
+            return Err(Error::CompareBlockedSignals(errno::Error::last()));
+        } else if ret > 0 {
+            return Err(Error::SignalAlreadyBlocked(num));
+        }
+    }
+    Ok(())
+}
+
+/// Unmasks given signal.
+pub fn unblock_signal(num: c_int) -> SignalResult<()> {
+    let sigset = create_sigset(&[num]).map_err(Error::CreateSigset)?;
+
+    // Safe - return value is checked.
+    let ret = unsafe { pthread_sigmask(SIG_UNBLOCK, &sigset, null_mut()) };
+    if ret < 0 {
+        return Err(Error::UnblockSignal(errno::Error::last()));
+    }
+    Ok(())
+}
+
 /// Trait for threads that can be signalled via `pthread_kill`.
 ///
 /// Note that this is only useful for signals between SIGRTMIN and SIGRTMAX because these are
@@ -59,17 +144,17 @@ pub unsafe fn register_signal_handler(num: u8, handler: extern "C" fn() -> ()) -
 pub unsafe trait Killable {
     fn pthread_handle(&self) -> pthread_t;
 
-    /// Sends the signal `num + SIGRTMIN` to this killable thread.
+    /// Sends the signal `num` to this killable thread.
     ///
-    /// The value of `num + SIGRTMIN` must not exceed `SIGRTMAX`.
-    fn kill(&self, num: u8) -> Result<()> {
+    /// The value of `num` must be within [`SIGRTMIN`, `SIGRTMAX`] range.
+    fn kill(&self, num: c_int) -> errno::Result<()> {
         if !valid_signal_num(num) {
-            return Err(Error::new(EINVAL));
+            return Err(errno::Error::new(EINVAL));
         }
 
         // Safe because we ensure we are using a valid pthread handle, a valid signal number, and
         // check the return result.
-        let ret = unsafe { pthread_kill(self.pthread_handle(), (num as i32) + SIGRTMIN()) };
+        let ret = unsafe { pthread_kill(self.pthread_handle(), num) };
         if ret < 0 {
             return errno_result();
         }
diff --git a/sys_util/src/signalfd.rs b/sys_util/src/signalfd.rs
index 8055ec4..dc08038 100644
--- a/sys_util/src/signalfd.rs
+++ b/sys_util/src/signalfd.rs
@@ -7,27 +7,21 @@ use std::result;
 use std::fs::File;
 use std::os::raw::c_int;
 use std::os::unix::io::{AsRawFd, FromRawFd, RawFd};
-use std::ptr::null_mut;
 
-use libc::{read, sigaddset, sigemptyset, sigismember, sigset_t, c_void, signalfd,
-           signalfd_siginfo, pthread_sigmask};
-use libc::{EAGAIN, SIG_BLOCK, SIG_UNBLOCK, SFD_NONBLOCK, SFD_CLOEXEC};
+use libc::{read, c_void, signalfd, signalfd_siginfo};
+use libc::{EAGAIN, SFD_NONBLOCK, SFD_CLOEXEC};
 
 use errno;
-use errno::errno_result;
+use signal;
 
 #[derive(Debug)]
 pub enum Error {
-    /// Couldn't create a sigset from the given signal.
+    /// Failed to construct sigset when creating signalfd.
     CreateSigset(errno::Error),
     /// Failed to create a new signalfd.
     CreateSignalFd(errno::Error),
-    /// The wrapped signal has already been blocked.
-    SignalAlreadyBlocked(c_int),
-    /// Failed to check if the requested signal is in the blocked set already.
-    CompareBlockedSignals(errno::Error),
-    /// The signal could not be blocked.
-    BlockSignal(errno::Error),
+    /// Failed to block the signal when creating signalfd.
+    CreateBlockSignal(signal::Error),
     /// Unable to read from signalfd.
     SignalFdRead(errno::Error),
     /// Signalfd could be read, but didn't return a full siginfo struct.
@@ -44,65 +38,33 @@ pub type Result<T> = result::Result<T, Error>;
 pub struct SignalFd {
     signalfd: File,
     signal: c_int,
-    sigset: sigset_t,
 }
 
 impl SignalFd {
-    fn create_sigset(signal: c_int) -> errno::Result<sigset_t> {
-        // sigset will actually be initialized by sigemptyset below.
-        let mut sigset: sigset_t = unsafe { mem::zeroed() };
-
-        // Safe - return value is checked.
-        let ret = unsafe { sigemptyset(&mut sigset as *mut sigset_t) };
-        if ret < 0 {
-            return errno_result();
-        }
-
-        let ret = unsafe { sigaddset(&mut sigset as *mut sigset_t, signal) };
-        if ret < 0 {
-            return errno_result();
-        }
-        Ok(sigset)
-    }
-
     /// Creates a new SignalFd for the given signal, blocking the normal handler
     /// for the signal as well. Since we mask out the normal handler, this is
     /// a risky operation - signal masking will persist across fork and even
     /// **exec** so the user of SignalFd should think long and hard about
     /// when to mask signals.
     pub fn new(signal: c_int) -> Result<SignalFd> {
-        // This unsafe block will create a signalfd that watches for the
-        // supplied signal, and then block the normal handler. At each
-        // step, we check return values.
-        unsafe {
-            let sigset = SignalFd::create_sigset(signal)
-                .map_err(Error::CreateSigset)?;
-            let fd = signalfd(-1, &sigset, SFD_CLOEXEC | SFD_NONBLOCK);
-            if fd < 0 {
-                return Err(Error::CreateSignalFd(errno::Error::last()));
-            }
+        let sigset = signal::create_sigset(&[signal]).map_err(Error::CreateSigset)?;
 
-            // Mask out the normal handler for the signal.
-            let mut old_sigset: sigset_t = mem::zeroed();
-            let ret = pthread_sigmask(SIG_BLOCK, &sigset, &mut old_sigset as *mut sigset_t);
-            if ret < 0 {
-                return Err(Error::BlockSignal(errno::Error::last()));
-            }
+        // This is safe as we check the return value and know that fd is valid.
+        let fd = unsafe { signalfd(-1, &sigset, SFD_CLOEXEC | SFD_NONBLOCK) };
+        if fd < 0 {
+            return Err(Error::CreateSignalFd(errno::Error::last()));
+        }
 
-            let ret = sigismember(&old_sigset, signal);
-            if ret < 0 {
-                return Err(Error::CompareBlockedSignals(errno::Error::last()));
-            } else if ret > 0 {
-                return Err(Error::SignalAlreadyBlocked(signal));
-            }
+        // Mask out the normal handler for the signal.
+        signal::block_signal(signal).map_err(Error::CreateBlockSignal)?;
 
-            // This is safe because we checked fd for success and know the
-            // kernel gave us an fd that we own.
+        // This is safe because we checked fd for success and know the
+        // kernel gave us an fd that we own.
+        unsafe {
             Ok(SignalFd {
-                   signalfd: File::from_raw_fd(fd),
-                   signal: signal,
-                   sigset: sigset,
-               })
+                signalfd: File::from_raw_fd(fd),
+                signal: signal,
+            })
         }
     }
 
@@ -150,12 +112,9 @@ impl Drop for SignalFd {
     fn drop(&mut self) {
         // This is thread-safe and safe in the sense that we're doing what
         // was promised - unmasking the signal when we go out of scope.
-        let ret =
-            unsafe { pthread_sigmask(SIG_UNBLOCK, &mut self.sigset as *mut sigset_t, null_mut()) };
-
-        // drop can't return a Result, so just print an error to syslog.
-        if ret < 0 {
-            error!("signalfd failed to unblock signal {}: {}", self.signal, ret);
+        let res = signal::unblock_signal(self.signal);
+        if let Err(e) = res {
+            error!("signalfd failed to unblock signal {}: {:?}", self.signal, e);
         }
     }
 }
@@ -164,7 +123,7 @@ impl Drop for SignalFd {
 mod tests {
     use super::*;
 
-    use libc::{raise, sigismember};
+    use libc::{raise, sigset_t, sigismember, pthread_sigmask};
     use signal::SIGRTMIN;
     use std::ptr::null;