Signal handler for SIGBUS, SIGSEGV

CHANGELOG.md

@@ -6,6 +6,8 @@
 
 - Added an experimental swagger definition that includes the specification for
   the vsock API call.
+- Added a signal handler for `SIGBUS` and `SIGSEGV` that immediately terminates
+  the process upon intercepting the signal.
 
 ## [0.16.0]
 

src/main.rs

@@ -30,6 +30,7 @@ use api_server::{ApiServer, Error};
 use fc_util::validators::validate_instance_id;
 use logger::{Metric, LOGGER, METRICS};
 use mmds::MMDS;
+use vmm::signal_handler::register_signal_handlers;
 use vmm::vmm_config::instance_info::{InstanceInfo, InstanceState};
 
 const DEFAULT_API_SOCK_PATH: &str = "/tmp/firecracker.socket";
@@ -40,11 +41,10 @@ fn main() {
         .preinit(Some(DEFAULT_INSTANCE_ID.to_string()))
         .expect("Failed to register logger");
 
-    if let Err(e) = vmm::setup_sigsys_handler() {
-        error!("Failed to register signal handler: {}", e);
+    if let Err(e) = register_signal_handlers() {
+        error!("Failed to register signal handlers: {}", e);
         process::exit(i32::from(vmm::FC_EXIT_CODE_GENERIC_ERROR));
     }
-
     // Start firecracker by setting up a panic hook, which will be called before
     // terminating as we're building with panic = "abort".
     // It's worth noting that the abort is caused by sending a SIG_ABORT signal to the process.

sys_util/src/signal.rs

@@ -7,34 +7,18 @@
 
 use super::SyscallReturnCode;
 use libc::{
-    c_int, c_void, pthread_kill, pthread_t, sigaction, siginfo_t, EINVAL, SA_SIGINFO, SIGHUP,
-    SIGSYS,
+    c_int, c_void, pthread_kill, pthread_t, sigaction, sigfillset, siginfo_t, sigset_t, EINVAL,
+    SIGHUP, SIGSYS,
 };
 use std::io;
 use std::mem;
 use std::os::unix::thread::JoinHandleExt;
+use std::ptr::null_mut;
 use std::thread::JoinHandle;
 
-type SiginfoHandler = extern "C" fn(num: c_int, info: *mut siginfo_t, _unused: *mut c_void) -> ();
-
-pub enum SignalHandler {
-    Siginfo(SiginfoHandler),
-    // TODO add a`SimpleHandler` when `libc` adds `sa_handler` support to `sigaction`.
-}
-
-/// Fills a `sigaction` structure from of the signal handler.
-impl Into<sigaction> for SignalHandler {
-    fn into(self) -> sigaction {
-        let mut act: sigaction = unsafe { mem::zeroed() };
-        match self {
-            SignalHandler::Siginfo(function) => {
-                act.sa_flags = SA_SIGINFO;
-                act.sa_sigaction = function as *const () as usize;
-            }
-        }
-        act
-    }
-}
+/// Type that represents a signal handler function.
+pub type SignalHandler =
+    extern "C" fn(num: c_int, info: *mut siginfo_t, _unused: *mut c_void) -> ();
 
 extern "C" {
     fn __libc_current_sigrtmin() -> c_int;
@@ -53,38 +37,130 @@ fn SIGRTMAX() -> c_int {
     unsafe { __libc_current_sigrtmax() }
 }
 
-/// Verifies that a signal number is valid.
+/// Verifies that a signal number is valid when sent to a vCPU.
 ///
-/// VCPU signals need to have values enclosed within the OS limits for realtime signals,
-/// and the remaining ones need to be between the minimum (SIGHUP) and maximum (SIGSYS) values.
+/// VCPU signals need to have values enclosed within the OS limits for realtime signals.
+/// Returns either `Ok(num)` or `Err(EINVAL)`.
 ///
-/// Will return Ok(num) or Err(EINVAL).
-pub fn validate_signal_num(num: c_int, for_vcpu: bool) -> io::Result<c_int> {
-    if for_vcpu {
-        let actual_num = num + SIGRTMIN();
-        if actual_num <= SIGRTMAX() {
-            return Ok(actual_num);
-        }
-    } else if SIGHUP <= num && num <= SIGSYS {
-        return Ok(num);
+/// # Arguments
+///
+/// * `signum`: signal number.
+///
+fn validate_vcpu_signal_num(signum: c_int) -> io::Result<c_int> {
+    let actual_num = signum + SIGRTMIN();
+    if actual_num <= SIGRTMAX() {
+        Ok(actual_num)
+    } else {
+        Err(io::Error::from_raw_os_error(EINVAL))
     }
-    Err(io::Error::from_raw_os_error(EINVAL))
 }
 
-/// Registers `handler` as the signal handler of signum `num`.
+/// Verifies that a signal number is valid when sent to the process.
 ///
-/// Uses `sigaction` to register the handler.
+/// Signals can take values between `SIGHUB` and `SIGSYS`.
+/// Returns either `Ok(num)` or `Err(EINVAL)`.
+///
+/// # Arguments
+///
+/// * `signum`: signal number.
+///
+fn validate_signal_num(num: c_int) -> io::Result<c_int> {
+    if num >= SIGHUP && num <= SIGSYS {
+        Ok(num)
+    } else {
+        Err(io::Error::from_raw_os_error(EINVAL))
+    }
+}
+
+/// Registers `handler` as the vCPU's signal handler of `signum`.
 ///
 /// 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: i32,
+///
+/// # Arguments
+///
+/// * `signum`: signal number.
+/// * `handler`: signal handler functor.
+///
+/// # Example
+///
+/// ```
+/// extern crate libc;
+/// extern crate sys_util;
+///
+/// use libc::{c_int, c_void, raise, siginfo_t};
+/// use sys_util::register_vcpu_signal_handler;
+///
+/// extern "C" fn handle_signal(_: c_int, _: *mut siginfo_t, _: *mut c_void) {}
+/// extern "C" { fn __libc_current_sigrtmin() -> c_int; }
+///
+/// fn main() {
+///     // Register dummy signal handler for `SIGRTMIN`.
+///     assert!(unsafe { register_vcpu_signal_handler(0, handle_signal).is_ok() });
+///     // Raise `SIGRTMIN`.
+///     unsafe { raise(__libc_current_sigrtmin()); }
+///     // Assert that the process is still alive.
+///     assert!(true);
+/// }
+/// ```
+///
+pub unsafe fn register_vcpu_signal_handler(
+    signum: c_int,
     handler: SignalHandler,
-    for_vcpu: bool,
 ) -> io::Result<()> {
-    let num = validate_signal_num(num, for_vcpu)?;
-    let act: sigaction = handler.into();
-    SyscallReturnCode(sigaction(num, &act, ::std::ptr::null_mut())).into_empty_result()
+    let num = validate_vcpu_signal_num(signum)?;
+    // Safe, because this is a POD struct.
+    let mut sigact: sigaction = mem::zeroed();
+    sigact.sa_flags = libc::SA_SIGINFO;
+    sigact.sa_sigaction = handler as usize;
+    SyscallReturnCode(sigaction(num, &sigact, null_mut())).into_empty_result()
+}
+
+/// Registers `handler` as the process' signal handler of `signum`.
+///
+/// # Arguments
+///
+/// * `signum`: signal number.
+/// * `handler`: signal handler functor.
+///
+/// # Example
+///
+/// ```
+/// extern crate libc;
+/// extern crate sys_util;
+///
+/// use std::sync::atomic::{AtomicBool, Ordering, ATOMIC_BOOL_INIT};
+/// use libc::{c_int, c_void, raise, siginfo_t, SIGUSR1};
+/// use sys_util::register_signal_handler;
+///
+/// static HANDLER_CALLED: AtomicBool = ATOMIC_BOOL_INIT;
+/// extern "C" fn handle_signal(_: c_int, _: *mut siginfo_t, _: *mut c_void) {
+///     HANDLER_CALLED.store(true, Ordering::SeqCst);
+/// }
+///
+/// fn main() {
+///     assert!(unsafe { register_signal_handler(SIGUSR1, handle_signal).is_ok() });
+///     unsafe { raise(SIGUSR1); }
+///     assert!(HANDLER_CALLED.load(Ordering::SeqCst));
+/// }
+/// ```
+///
+pub fn register_signal_handler(signum: c_int, handler: SignalHandler) -> Result<(), io::Error> {
+    let num = validate_signal_num(signum)?;
+    // Safe, because this is a POD struct.
+    let mut sigact: sigaction = unsafe { mem::zeroed() };
+    sigact.sa_flags = libc::SA_SIGINFO;
+    sigact.sa_sigaction = handler as usize;
+
+    // We set all the bits of sa_mask, so all signals are blocked on the current thread while the
+    // SIGSYS handler is executing. Safe because the parameter is valid and we check the return
+    // value.
+    if unsafe { sigfillset(&mut sigact.sa_mask as *mut sigset_t) } < 0 {
+        return Err(io::Error::last_os_error());
+    }
+
+    // Safe because the parameters are valid and we check the return value.
+    unsafe { SyscallReturnCode(sigaction(num, &sigact, null_mut())).into_empty_result() }
 }
 
 /// Trait for threads that can be signalled via `pthread_kill`.
@@ -101,7 +177,7 @@ pub unsafe trait Killable {
     ///
     /// The value of `num + SIGRTMIN` must not exceed `SIGRTMAX`.
     fn kill(&self, num: i32) -> io::Result<()> {
-        let num = validate_signal_num(num, true)?;
+        let num = validate_vcpu_signal_num(num)?;
 
         // Safe because we ensure we are using a valid pthread handle, a valid signal number, and
         // check the return result.
@@ -119,10 +195,11 @@ unsafe impl<T> Killable for JoinHandle<T> {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use libc;
     use std::thread;
     use std::time::Duration;
 
+    use libc::SIGSYS;
+
     static mut SIGNAL_HANDLER_CALLED: bool = false;
 
     extern "C" fn handle_signal(_: c_int, _: *mut siginfo_t, _: *mut c_void) {
@@ -135,25 +212,10 @@ mod tests {
     fn test_register_signal_handler() {
         unsafe {
             // testing bad value
-            assert!(register_signal_handler(
-                SIGRTMAX(),
-                SignalHandler::Siginfo(handle_signal),
-                true
-            )
-            .is_err());
-            format!(
-                "{:?}",
-                register_signal_handler(SIGRTMAX(), SignalHandler::Siginfo(handle_signal), true)
-            );
-            assert!(
-                register_signal_handler(0, SignalHandler::Siginfo(handle_signal), true).is_ok()
-            );
-            assert!(register_signal_handler(
-                libc::SIGSYS,
-                SignalHandler::Siginfo(handle_signal),
-                false
-            )
-            .is_ok());
+            assert!(register_vcpu_signal_handler(SIGRTMAX(), handle_signal).is_err());
+            assert!(register_vcpu_signal_handler(0, handle_signal).is_ok());
+            assert!(register_signal_handler(SIGSYS, handle_signal).is_ok());
+            assert!(register_signal_handler(SIGSYS + 1, handle_signal).is_err());
         }
     }
 
@@ -168,7 +230,7 @@ mod tests {
         // be brought down when the signal is received, as part of the default behaviour. Signal
         // handlers are global, so we install this before starting the thread.
         unsafe {
-            register_signal_handler(0, SignalHandler::Siginfo(handle_signal), true)
+            register_vcpu_signal_handler(0, handle_signal)
                 .expect("failed to register vcpu signal handler");
         }
 

tests/integration_tests/build/test_licenses.py

@@ -6,8 +6,9 @@
 
 import pytest
 
+AMAZON_COPYRIGHT_YEARS = (2018, 2019)
 AMAZON_COPYRIGHT = (
-    "Copyright 2018 Amazon.com, Inc. or its affiliates. All Rights Reserved."
+    "Copyright {} Amazon.com, Inc. or its affiliates. All Rights Reserved."
     )
 AMAZON_LICENSE = (
         "SPDX-License-Identifier: Apache-2.0"
@@ -28,7 +29,14 @@
 EXCLUDED_DIRECTORIES = ((os.path.join(os.getcwd(), 'build')))
 
 
-def validate_license(filename):
+def _has_amazon_copyright(string):
+    for year in AMAZON_COPYRIGHT_YEARS:
+        if AMAZON_COPYRIGHT.format(year) in string:
+            return True
+    return False
+
+
+def _validate_license(filename):
     """Validate licenses in all .rs, .py. and .sh file.
 
     Python and Rust files should have the licenses on the first 2 lines
@@ -45,7 +53,7 @@ def validate_license(filename):
             copy = file.readline()
             local_license = file.readline()
             has_amazon_copyright = (
-                    AMAZON_COPYRIGHT in copy and
+                    _has_amazon_copyright(copy) and
                     AMAZON_LICENSE in local_license
             )
             has_chromium_copyright = (
@@ -70,5 +78,5 @@ def test_for_valid_licenses():
     for subdir, _, files in os.walk(os.getcwd()):
         for file in files:
             filepath = os.path.join(subdir, file)
-            assert validate_license(filepath) is True, \
+            assert _validate_license(filepath) is True, \
                 "%s has invalid license" % filepath

tests/integration_tests/functional/test_signals.py

@@ -0,0 +1,56 @@
+# Copyright 2019 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+# SPDX-License-Identifier: Apache-2.0
+"""Tests scenarios for Firecracker signal handling."""
+
+import os
+from signal import SIGBUS, SIGSEGV
+from time import sleep
+
+import pytest
+
+import host_tools.logging as log_tools
+
+
+@pytest.mark.parametrize(
+    "signum",
+    [SIGBUS, SIGSEGV]
+)
+def test_sigbus_sigsegv(test_microvm_with_api, signum):
+    """Test signal handling for `SIGBUS` and `SIGSEGV`."""
+    test_microvm = test_microvm_with_api
+    test_microvm.spawn()
+
+    # We don't need to monitor the memory for this test.
+    test_microvm.memory_events_queue = None
+
+    test_microvm.basic_config()
+
+    # Configure logging.
+    log_fifo_path = os.path.join(test_microvm.path, 'log_fifo')
+    metrics_fifo_path = os.path.join(test_microvm.path, 'metrics_fifo')
+    log_fifo = log_tools.Fifo(log_fifo_path)
+    metrics_fifo = log_tools.Fifo(metrics_fifo_path)
+
+    response = test_microvm.logger.put(
+        log_fifo=test_microvm.create_jailed_resource(log_fifo.path),
+        metrics_fifo=test_microvm.create_jailed_resource(metrics_fifo.path),
+        level='Error',
+        show_level=False,
+        show_log_origin=False
+    )
+    assert test_microvm.api_session.is_status_no_content(response.status_code)
+
+    test_microvm.start()
+    firecracker_pid = int(test_microvm.jailer_clone_pid)
+
+    sleep(0.5)
+    os.kill(firecracker_pid, signum)
+
+    msg = 'Shutting down VM after intercepting signal {}'.format(signum)
+    lines = log_fifo.sequential_reader(5)
+    msg_found = False
+    for line in lines:
+        if msg in line:
+            msg_found = True
+            break
+    assert msg_found