vmm: add signal handler for SIGBUS, SIGSEGV

* rework signal handling code to handle more than SIGSYS.
* add a minimal representation of the siginfo_t structure that mirrors the
  union-model defined in libc. This works around the missing functionality
  in the libc crate, where siginfo_t is defined using a generic padding field.
  The union solution allows us to extract fields from the siginfo struct
  without pesky offset calculations and transmutations.

Signed-off-by: Alexandra Iordache <[email protected]>

Author: Alexandra Iordache

src/main.rs

@@ -40,7 +40,7 @@ fn main() {
         .preinit(Some(DEFAULT_INSTANCE_ID.to_string()))
         .expect("Failed to register logger");
 
-    if let Err(e) = vmm::setup_sigsys_handler() {
+    if let Err(e) = vmm::setup_signal_handlers() {
         error!("Failed to register signal handler: {}", e);
         process::exit(i32::from(vmm::FC_EXIT_CODE_GENERIC_ERROR));
     }

vmm/src/lib.rs

@@ -39,6 +39,8 @@ extern crate sys_util;
 /// Syscalls allowed through the seccomp filter.
 pub mod default_syscalls;
 mod device_manager;
+/// Generic signal handling utilities.
+mod signal;
 /// Signal handling utilities for seccomp violations.
 mod sigsys_handler;
 /// Wrappers over structures used to configure the VMM.
@@ -77,7 +79,6 @@ use memory_model::{GuestAddress, GuestMemory};
 use net_util::TapError;
 #[cfg(target_arch = "aarch64")]
 use serde_json::Value;
-pub use sigsys_handler::setup_sigsys_handler;
 use sys_util::{EventFd, Terminal};
 use vmm_config::boot_source::{BootSourceConfig, BootSourceConfigError};
 use vmm_config::drive::{BlockDeviceConfig, BlockDeviceConfigs, DriveError};
@@ -114,6 +115,10 @@ pub const FC_EXIT_CODE_GENERIC_ERROR: u8 = 1;
 pub const FC_EXIT_CODE_UNEXPECTED_ERROR: u8 = 2;
 /// Firecracker was shut down after intercepting a restricted system call.
 pub const FC_EXIT_CODE_BAD_SYSCALL: u8 = 148;
+/// Firecracker was shut down after intercepting `SIGBUS`.
+pub const FC_EXIT_CODE_SIGBUS: u8 = 149;
+/// Firecracker was shut down after intercepting `SIGSEGV`.
+pub const FC_EXIT_CODE_SIGSEGV: u8 = 150;
 
 /// Errors associated with the VMM internal logic. These errors cannot be generated by direct user
 /// input, but can result from bad configuration of the host (for example if Firecracker doesn't
@@ -2025,6 +2030,17 @@ pub fn start_vmm_thread(
         .expect("VMM thread spawn failed.")
 }
 
+/// Sets up appropriate signal handlers for all signals relevant to Firecracker.
+///
+pub fn setup_signal_handlers() -> result::Result<(), io::Error> {
+    signal::setup_signal_handler(&vec![libc::SIGSYS], sigsys_handler::sigsys_handler)?;
+    signal::setup_signal_handler(
+        &vec![libc::SIGBUS, libc::SIGSEGV],
+        signal::sigbus_sigsegv_handler,
+    )?;
+    Ok(())
+}
+
 #[cfg(test)]
 mod tests {
     extern crate tempfile;

vmm/src/signal.rs

@@ -0,0 +1,230 @@
+// Copyright 2018 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+extern crate logger;
+extern crate sys_util;
+
+use std::io;
+use std::mem;
+use std::ptr::null_mut;
+use std::result::Result;
+
+use libc::{
+    _exit, c_int, c_uint, c_void, sigaction, sigfillset, siginfo_t, sigset_t, SA_SIGINFO, SIGBUS,
+    SIGSEGV,
+};
+
+use logger::LOGGER;
+
+type SiginfoHandler = extern "C" fn(num: c_int, info: *mut siginfo_t, _unused: *mut c_void) -> ();
+
+// `SIGBUS` codes.
+// Hardware memory error consumed on a machine check: action required.
+const BUS_MCEERR_AR: c_int = 4;
+// Hardware memory error detected in process but not consumed: action optional.
+const BUS_MCEERR_AO: c_int = 5;
+
+// `SIGSEGV` codes.
+// Failed address bound checks.
+const SEGV_BNDERR: c_int = 3;
+// Failed protection key checks.
+const SEGV_PKUERR: c_int = 4;
+
+/// Address bounds at time of fault.
+/// See https://elixir.bootlin.com/linux/v4.14/source/include/uapi/asm-generic/siginfo.h
+#[repr(C)]
+#[derive(Copy, Clone)]
+pub(crate) struct _si_addr_bnd_t {
+    /// Lower bound.
+    pub(crate) _lower: *const c_void,
+    /// Upper bound.
+    pub(crate) _upper: *const c_void,
+}
+
+/// Representation of the fault stats for different `SIGSEGV` codes.
+/// See https://elixir.bootlin.com/linux/v4.14/source/include/uapi/asm-generic/siginfo.h
+#[repr(C)]
+#[derive(Copy, Clone)]
+pub(crate) union _si_fault_t {
+    /// Used when `si_code` == `SEGV_BNDERR`.
+    pub(crate) _addr_bnd: _si_addr_bnd_t,
+    /// Used when `si_code` == `SEGV_PKUERR`.
+    pub(crate) _pkey: c_uint,
+}
+
+/// Stats filled in for `SIGILL`, `SIGFPE`, `SIGSEGV`, `SIGBUS`.
+/// See https://elixir.bootlin.com/linux/v4.14/source/include/uapi/asm-generic/siginfo.h
+#[repr(C)]
+#[derive(Copy, Clone)]
+pub(crate) struct _sigfault_t {
+    /// Faulting instruction / memory reference address.
+    pub(crate) _si_addr: *const c_void,
+    /// Valid LSB of the reported address.
+    pub(crate) _si_addr_lsb: i16,
+    /// Fault stats.
+    pub(crate) _si_stats: _si_fault_t,
+}
+
+/// Stats filled in for `SIGSYS`.
+/// See https://elixir.bootlin.com/linux/v4.14/source/include/uapi/asm-generic/siginfo.h
+#[repr(C)]
+#[derive(Copy, Clone)]
+pub(crate) struct _sigsys_t {
+    /// Call address.
+    pub(crate) _call_addr: *const c_void,
+    /// Offending syscall number.
+    pub(crate) _syscall: c_int,
+    /// Architecture identifier.
+    pub(crate) _arch: c_uint,
+}
+
+/// Union of possible additional stats returned in the `siginfo` struct.
+/// See https://elixir.bootlin.com/linux/v4.14/source/include/uapi/asm-generic/siginfo.h
+#[repr(C)]
+#[derive(Copy, Clone)]
+pub(crate) union _si_fields_t {
+    /// Fault info. Filled in for `SIGSEGV`, `SIGBUS`, `SIGILL`, `SIGFPE`.
+    pub(crate) _si_sigfault: _sigfault_t,
+    /// `SIGSYS` info. Filled in for seccomp faults.
+    pub(crate) _si_sigsys: _sigsys_t,
+    /// Padding.
+    _pad: [c_int; 29],
+}
+
+/// Representation of the `siginfo` struct with its relevant fields.
+/// See https://elixir.bootlin.com/linux/v4.14/source/include/uapi/asm-generic/siginfo.h
+#[repr(C)]
+#[derive(Copy, Clone)]
+pub(crate) struct _si_siginfo_t {
+    /// Signal number.
+    pub(crate) si_signo: c_int,
+    /// If non-zero, errno value associated with the signal.
+    pub(crate) si_errno: c_int,
+    /// Signal code.
+    pub(crate) si_code: c_int,
+    /// Additional fields.
+    pub(crate) si_fields: _si_fields_t,
+}
+
+/// Sets up the specified handler for the signals.
+///
+/// # Arguments
+///
+/// * `signals` - vector of signals to be handled.
+/// * `handler` - signal handler function.
+///
+pub(crate) fn setup_signal_handler(
+    signals: &Vec<c_int>,
+    handler: SiginfoHandler,
+) -> Result<(), io::Error> {
+    // Safe, because this is a POD struct.
+    let mut sigact: sigaction = unsafe { mem::zeroed() };
+    sigact.sa_flags = 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
+    // signal 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());
+    }
+
+    for signal in signals.iter() {
+        // Safe because the parameters are valid and we check the return value.
+        if unsafe { sigaction(*signal, &sigact, null_mut()) } < 0 {
+            return Err(io::Error::last_os_error());
+        }
+    }
+
+    Ok(())
+}
+
+/// Handles `SIGBUS` and `SIGSEGV`.
+///
+/// Logs all the available information on the fault that occurred and exits the process.
+///
+/// # Arguments
+///
+/// * `num` - signal number.
+/// * `info` - signal information filled in by the kernel.
+/// * `c_void` - signal context. Unused.
+///
+pub(crate) extern "C" fn sigbus_sigsegv_handler(
+    num: c_int,
+    info: *mut siginfo_t,
+    _unused: *mut c_void,
+) {
+    let siginfo = info as *mut _si_siginfo_t;
+    // Safe because we dereference a valid value.
+    let si_signo = unsafe { (*siginfo).si_signo };
+    let si_code = unsafe { (*siginfo).si_code };
+
+    // Sanity check. The condition should never be true.
+    if num != si_signo || (num != SIGBUS && num != SIGSEGV) {
+        // Safe because we're terminating the process anyway.
+        unsafe { _exit(i32::from(super::FC_EXIT_CODE_UNEXPECTED_ERROR)) };
+    }
+
+    // `SIGSEGV` and `SIGBUS` fill in `si_addr` with the address of the fault.
+    // http://man7.org/linux/man-pages/man2/sigaction.2.html
+    // Safe because we dereference a valid value.
+    let si_addr = unsafe { (*siginfo).si_fields._si_sigfault._si_addr as usize };
+    error!(
+        "Caught signal {}. Code: {}. Fault address: {:x?}",
+        si_signo, si_code, si_addr
+    );
+
+    match si_signo {
+        SIGBUS => {
+            match si_code {
+                // `BUS_MCEERR_AO` and `BUS_MCEERR_AR` also fill in `si_addr_lsb`.
+                // This field indicates the LSB of the reported address and therefore the extent of
+                // the corruption.  For example, if a full page was corrupted, `si_addr_lsb` contains
+                // `log2(sysconf(_SC_PAGESIZE))`.
+                BUS_MCEERR_AO | BUS_MCEERR_AR => {
+                    // Safe because we dereference a valid value.
+                    let si_addr_lsb = unsafe { (*siginfo).si_fields._si_sigfault._si_addr_lsb };
+                    error!("LSB of the reported address: {:x?}", si_addr_lsb);
+                }
+                _ => (),
+            }
+        }
+        SIGSEGV => {
+            match si_code {
+                SEGV_BNDERR => {
+                    // The `SEGV_BNDERR` suberror of `SIGSEGV` populates `si_lower` and `si_upper`.
+                    // Safe because we dereference a valid value.
+                    let addr_bnd = unsafe { (*siginfo).si_fields._si_sigfault._si_stats._addr_bnd };
+                    error!(
+                        "Failed address bound checks. Bounds: lower {:x?} upper {:x?}",
+                        addr_bnd._lower, addr_bnd._upper
+                    );
+                }
+                SEGV_PKUERR => {
+                    // The `SEGV_PKUERR` suberror of `SIGSEGV` populates `si_pkey`.
+                    // Safe because we dereference a valid value.
+                    let pkey = unsafe { (*siginfo).si_fields._si_sigfault._si_stats._pkey };
+                    error!("Failed protection key checks: {}", pkey);
+                }
+                _ => (),
+            }
+        }
+        _ => (),
+    }
+
+    // Log the metrics before exiting.
+    if let Err(e) = LOGGER.log_metrics() {
+        error!("Failed to log metrics while stopping: {}", e);
+    }
+
+    // Safe because we're terminating the process anyway. We don't actually do anything when
+    // running unit tests.
+    #[cfg(not(test))]
+    unsafe {
+        _exit(i32::from(match si_signo {
+            SIGBUS => super::FC_EXIT_CODE_SIGBUS,
+            SIGSEGV => super::FC_EXIT_CODE_SIGSEGV,
+            _ => super::FC_EXIT_CODE_UNEXPECTED_ERROR,
+        }))
+    };
+}

vmm/src/sigsys_handler.rs

@@ -4,57 +4,20 @@
 extern crate logger;
 extern crate sys_util;
 
-use std::io;
-use std::mem;
-use std::ptr::null_mut;
-use std::result::Result;
-
-use libc::{sigaction, sigfillset, sigset_t};
-
 use logger::{Metric, LOGGER, METRICS};
-
-// The offset of `si_syscall` (offending syscall identifier) within the siginfo structure
-// expressed as an `(u)int*`.
-// Offset `6` for an `i32` field means that the needed information is located at `6 * sizeof(i32)`.
-// See /usr/include/linux/signal.h for the C struct definition.
-// See https://github.com/rust-lang/libc/issues/716 for why the offset is different in Rust.
-const SI_OFF_SYSCALL: isize = 6;
+use signal::_si_siginfo_t;
 
 const SYS_SECCOMP_CODE: i32 = 1;
 
-// This no longer relies on sys_util::register_signal_handler(), which is a lot weirder than it
-// should be (at least for this use case). Also, we want to change the sa_mask field of the
-// sigaction struct.
-/// Sets up the specified signal handler for `SIGSYS`.
-pub fn setup_sigsys_handler() -> Result<(), io::Error> {
-    // Safe, because this is a POD struct.
-    let mut sigact: sigaction = unsafe { mem::zeroed() };
-    sigact.sa_flags = libc::SA_SIGINFO;
-    sigact.sa_sigaction = sigsys_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.
-    if unsafe { sigaction(libc::SIGSYS, &sigact, null_mut()) } < 0 {
-        return Err(io::Error::last_os_error());
-    }
-
-    Ok(())
-}
-
-extern "C" fn sigsys_handler(
+pub(crate) extern "C" fn sigsys_handler(
     num: libc::c_int,
     info: *mut libc::siginfo_t,
     _unused: *mut libc::c_void,
 ) {
+    let siginfo = info as *mut _si_siginfo_t;
     // Safe because we're just reading some fields from a supposedly valid argument.
-    let si_signo = unsafe { (*info).si_signo };
-    let si_code = unsafe { (*info).si_code };
+    let si_signo = unsafe { (*siginfo).si_signo };
+    let si_code = unsafe { (*siginfo).si_code };
 
     // Sanity check. The condition should never be true.
     if num != si_signo || num != libc::SIGSYS || si_code != SYS_SECCOMP_CODE as i32 {
@@ -64,7 +27,8 @@ extern "C" fn sigsys_handler(
 
     // Other signals which might do async unsafe things incompatible with the rest of this
     // function are blocked due to the sa_mask used when registering the signal handler.
-    let syscall = unsafe { *(info as *const i32).offset(SI_OFF_SYSCALL) as usize };
+    // Safe because we dereference a valid value.
+    let syscall = unsafe { (*siginfo).si_fields._si_sigsys._syscall };
     METRICS.seccomp.num_faults.inc();
     error!(
         "Shutting down VM after intercepting a bad syscall ({}).",
@@ -85,6 +49,7 @@ extern "C" fn sigsys_handler(
 
 #[cfg(test)]
 mod tests {
+    use super::super::signal::setup_signal_handler;
     use super::*;
 
     use std::mem;
@@ -120,7 +85,7 @@ mod tests {
 
     #[test]
     fn test_signal_handler() {
-        assert!(setup_sigsys_handler().is_ok());
+        assert!(setup_signal_handler(&vec![libc::SIGSYS], sigsys_handler).is_ok());
 
         let filter = SeccompFilter::new(
             vec![