Merge pull request #812 from cgwalters/block-sigterm

Catch and ignore SIGTERM during update operations

Cargo.toml

@@ -44,6 +44,7 @@ serde_json = "^1.0"
 tempfile = "^3.14"
 widestring = "1.1.0"
 walkdir = "2.3.2"
+signal-hook-registry = "1.4.2"
 
 [profile.release]
 # We assume we're being delivered via e.g. RPM which supports split debuginfo

contrib/packaging/bootloader-update.service

@@ -6,6 +6,10 @@ Documentation=https://github.com/coreos/bootupd
 Type=oneshot
 ExecStart=/usr/bin/bootupctl update
 RemainAfterExit=yes
+# Keep this stuff in sync with SYSTEMD_ARGS_BOOTUPD in general
+PrivateNetwork=yes
+ProtectHome=yes
+KillMode=mixed
 MountFlags=slave
 
 [Install]

src/backend/statefile.rs

@@ -9,6 +9,25 @@ use std::fs::File;
 use std::io::prelude::*;
 use std::path::Path;
 
+/// Suppress SIGTERM while active
+// TODO: In theory we could record if we got SIGTERM and exit
+// on drop, but in practice we don't care since we're going to exit anyways.
+#[derive(Debug)]
+struct SignalTerminationGuard(signal_hook_registry::SigId);
+
+impl SignalTerminationGuard {
+    pub(crate) fn new() -> Result<Self> {
+        let signal = unsafe { signal_hook_registry::register(libc::SIGTERM, || {})? };
+        Ok(Self(signal))
+    }
+}
+
+impl Drop for SignalTerminationGuard {
+    fn drop(&mut self) {
+        signal_hook_registry::unregister(self.0);
+    }
+}
+
 impl SavedState {
     /// System-wide bootupd write lock (relative to sysroot).
     const WRITE_LOCK_PATH: &'static str = "run/bootupd-lock";
@@ -27,6 +46,7 @@ impl SavedState {
         lockfile.lock_exclusive()?;
         let guard = StateLockGuard {
             sysroot,
+            termguard: Some(SignalTerminationGuard::new()?),
             lockfile: Some(lockfile),
         };
         Ok(guard)
@@ -37,6 +57,7 @@ impl SavedState {
     pub(crate) fn unlocked(sysroot: openat::Dir) -> Result<StateLockGuard> {
         Ok(StateLockGuard {
             sysroot,
+            termguard: None,
             lockfile: None,
         })
     }
@@ -91,6 +112,8 @@ impl SavedState {
 pub(crate) struct StateLockGuard {
     pub(crate) sysroot: openat::Dir,
     #[allow(dead_code)]
+    termguard: Option<SignalTerminationGuard>,
+    #[allow(dead_code)]
     lockfile: Option<File>,
 }
 

src/cli/bootupctl.rs

@@ -6,16 +6,17 @@ use log::LevelFilter;
 use std::os::unix::process::CommandExt;
 use std::process::{Command, Stdio};
 
-static SYSTEMD_ARGS_BOOTUPD: &[&str] = &[
-    "--unit",
-    "bootupd",
-    "--property",
+static SYSTEMD_ARGS_BOOTUPD: &[&str] = &["--unit", "bootupd", "--pipe"];
+
+/// Keep these properties (isolation/runtime state) in sync with
+/// the systemd units in contrib/packaging/*.service
+static SYSTEMD_PROPERTIES: &[&str] = &[
     "PrivateNetwork=yes",
-    "--property",
     "ProtectHome=yes",
-    "--property",
+    // While only our main process during update catches SIGTERM, we don't
+    // want systemd to send it to other processes.
+    "KillMode=mixed",
     "MountFlags=slave",
-    "--pipe",
 ];
 
 /// `bootupctl` sub-commands.
@@ -167,6 +168,11 @@ fn ensure_running_in_systemd() -> Result<()> {
             .wait()?;
         let r = Command::new("systemd-run")
             .args(SYSTEMD_ARGS_BOOTUPD)
+            .args(
+                SYSTEMD_PROPERTIES
+                    .into_iter()
+                    .flat_map(|&v| ["--property", v]),
+            )
             .args(std::env::args())
             .exec();
         // If we got here, it's always an error