main: add seccomp-log-failures flag to command line

All cros-debug versions of crosvm enabled seccomp logging, which is now
broken on kernels <4.4 thanks to new minijail changes as explained in
the referenced BUG. This seems to be intended by the minijail folks as
the aim to improve the seccomp logging in part by changing its semantics
to logging failures without killing the violating process. In such a
world, crosvm should not as a compile time choice, enable logging, which
would amount to disabling some of the security. This change adds a
command line flag to emulate the old behavior for the purposes of
developer debugging, as long as that developer is running on a kernel
that supports the new minijail seccomp filter failure logging.

BUG=chromium:978998
TEST=USE=cros-debug emerge-eve crosvm && cros deploy eve crosvm
     then start crostini in UI

Change-Id: I98190a068a919929e466fe22d6d630b90a758336
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/1679380
Reviewed-by: Zach Reizner <zachr@chromium.org>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Tested-by: Zach Reizner <zachr@chromium.org>
Tested-by: kokoro <noreply+kokoro@google.com>
Commit-Queue: Zach Reizner <zachr@chromium.org>
Auto-Submit: Zach Reizner <zachr@chromium.org>

3 files changed, 24 insertions, 8 deletions

diff --git a/src/linux.rs b/src/linux.rs
index 48cbe52..8d4a4ae 100644
--- a/src/linux.rs
+++ b/src/linux.rs
@@ -270,7 +270,11 @@ impl AsRawFd for TaggedControlSocket {
     }
 }
 
-fn create_base_minijail(root: &Path, seccomp_policy: &Path) -> Result<Minijail> {
+fn create_base_minijail(
+    root: &Path,
+    log_failures: bool,
+    seccomp_policy: &Path,
+) -> Result<Minijail> {
     // All child jails run in a new user namespace without any users mapped,
     // they run as nobody unless otherwise configured.
     let mut j = Minijail::new().map_err(Error::DeviceJail)?;
@@ -289,8 +293,9 @@ fn create_base_minijail(root: &Path, seccomp_policy: &Path) -> Result<Minijail>
     // Use TSYNC only for the side effect of it using SECCOMP_RET_TRAP, which will correctly kill
     // the entire device process if a worker thread commits a seccomp violation.
     j.set_seccomp_filter_tsync();
-    #[cfg(debug_assertions)]
-    j.log_seccomp_filter_failures();
+    if log_failures {
+        j.log_seccomp_filter_failures();
+    }
     j.parse_seccomp_filters(seccomp_policy)
         .map_err(Error::DeviceJail)?;
     j.use_seccomp_filter();
@@ -308,7 +313,11 @@ fn simple_jail(cfg: &Config, policy: &str) -> Result<Option<Minijail>> {
             return Err(Error::PivotRootDoesntExist(pivot_root));
         }
         let policy_path: PathBuf = cfg.seccomp_policy_dir.join(policy);
-        Ok(Some(create_base_minijail(root_path, &policy_path)?))
+        Ok(Some(create_base_minijail(
+            root_path,
+            cfg.seccomp_log_failures,
+            &policy_path,
+        )?))
     } else {
         Ok(None)
     }
diff --git a/src/main.rs b/src/main.rs
index ee02b57..81f20a7 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -114,6 +114,7 @@ pub struct Config {
     shared_dirs: Vec<(PathBuf, String)>,
     sandbox: bool,
     seccomp_policy_dir: PathBuf,
+    seccomp_log_failures: bool,
     gpu: bool,
     software_tpm: bool,
     cras_audio: bool,
@@ -158,6 +159,7 @@ impl Default for Config {
             shared_dirs: Vec::new(),
             sandbox: !cfg!(feature = "default-no-sandbox"),
             seccomp_policy_dir: PathBuf::from(SECCOMP_POLICY_DIR),
+            seccomp_log_failures: false,
             cras_audio: false,
             cras_capture: false,
             null_audio: false,
@@ -597,6 +599,9 @@ fn set_argument(cfg: &mut Config, name: &str, value: Option<&str>) -> argument::
             // `value` is Some because we are in this match so it's safe to unwrap.
             cfg.seccomp_policy_dir = PathBuf::from(value.unwrap());
         }
+        "seccomp-log-failures" => {
+            cfg.seccomp_log_failures = true;
+        }
         "plugin" => {
             if cfg.executable_path.is_some() {
                 return Err(argument::Error::TooManyArguments(format!(
@@ -856,6 +861,7 @@ fn run_vm(args: std::env::Args) -> std::result::Result<(), ()> {
           Argument::value("shared-dir", "PATH:TAG",
                           "Directory to be shared with a VM as a source:tag pair. Can be given more than once."),
           Argument::value("seccomp-policy-dir", "PATH", "Path to seccomp .policy files."),
+          Argument::flag("seccomp-log-failures", "Instead of seccomp filter failures being fatal, they will be logged instead."),
           #[cfg(feature = "plugin")]
           Argument::value("plugin", "PATH", "Absolute path to plugin process to run under crosvm."),
           #[cfg(feature = "plugin")]
diff --git a/src/plugin/mod.rs b/src/plugin/mod.rs
index 1cb3e57..cf8332a 100644
--- a/src/plugin/mod.rs
+++ b/src/plugin/mod.rs
@@ -265,7 +265,7 @@ fn mmap_to_sys_err(e: MmapError) -> SysError {
     }
 }
 
-fn create_plugin_jail(root: &Path, seccomp_policy: &Path) -> Result<Minijail> {
+fn create_plugin_jail(root: &Path, log_failures: bool, seccomp_policy: &Path) -> Result<Minijail> {
     // All child jails run in a new user namespace without any users mapped,
     // they run as nobody unless otherwise configured.
     let mut j = Minijail::new().map_err(Error::CreateJail)?;
@@ -287,8 +287,9 @@ fn create_plugin_jail(root: &Path, seccomp_policy: &Path) -> Result<Minijail> {
     // Use TSYNC only for the side effect of it using SECCOMP_RET_TRAP, which will correctly kill
     // the entire plugin process if a worker thread commits a seccomp violation.
     j.set_seccomp_filter_tsync();
-    #[cfg(debug_assertions)]
-    j.log_seccomp_filter_failures();
+    if log_failures {
+        j.log_seccomp_filter_failures();
+    }
     j.parse_seccomp_filters(seccomp_policy)
         .map_err(Error::ParseSeccomp)?;
     j.use_seccomp_filter();
@@ -540,7 +541,7 @@ pub fn run_config(cfg: Config) -> Result<()> {
         }
 
         let policy_path = cfg.seccomp_policy_dir.join("plugin.policy");
-        let mut jail = create_plugin_jail(root_path, &policy_path)?;
+        let mut jail = create_plugin_jail(root_path, cfg.seccomp_log_failures, &policy_path)?;
 
         // Update gid map of the jail if caller provided supplemental groups.
         if !cfg.plugin_gid_maps.is_empty() {