use MASKED_EQ for all 32bit arguments

Author: pb8o

src/seccompiler/Cargo.toml

@@ -22,7 +22,7 @@ displaydoc = "0.2.5"
 libc = "0.2.168"
 serde = { version = "1.0.215", features = ["derive"] }
 serde_json = "1.0.133"
-thiserror = "2.0.3"
+thiserror = "2.0.6"
 zerocopy = { version = "0.8.11" }
 
 [lints]

src/seccompiler/src/lib.rs

@@ -121,7 +121,7 @@ pub fn compile_bpf(
             } else if let Some(rules) = &rule.args {
                 let comparators = rules
                     .iter()
-                    .map(|rule| rule.to_scmp_type(&arch, syscall))
+                    .map(|rule| rule.to_scmp_type())
                     .collect::<Vec<scmp_arg_cmp>>();
 
                 // SAFETY: Safe as all args are correct.

src/seccompiler/src/types.rs

@@ -1,4 +1,6 @@
 // Copyright 2024 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+// Copyright 2021 Sony Group Corporation
+//
 // SPDX-License-Identifier: Apache-2.0
 
 use std::collections::BTreeMap;
@@ -23,39 +25,54 @@ pub enum SeccompCmpOp {
     Ne,
 }
 
+/// Seccomp argument value length.
+#[derive(Clone, Debug, Deserialize, PartialEq)]
+#[serde(rename_all = "lowercase")]
+pub enum SeccompCmpArgLen {
+    /// Argument value length is 4 bytes.
+    Dword,
+    /// Argument value length is 8 bytes.
+    Qword,
+}
+
 /// Condition that syscall must match in order to satisfy a rule.
 #[derive(Debug, Deserialize)]
 pub struct SeccompCondition {
     pub index: u8,
     pub op: SeccompCmpOp,
     pub val: u64,
+    #[serde(rename = "type")]
+    pub val_len: SeccompCmpArgLen,
 }
 
 impl SeccompCondition {
-    pub fn to_scmp_type(&self, arch: &TargetArch, syscall: i32) -> scmp_arg_cmp {
-        // For `ioctls` we need to mask upper bits as musl
-        // sets them to 1, but libseccomp explicitly checks that they are 0.
-        // with 0x00000000FFFFFFFF mask upper bits are always 0.
-        let ioctl = match arch {
-            TargetArch::X86_64 => 16,
-            TargetArch::Aarch64 => 29,
-        };
+    pub fn to_scmp_type(&self) -> scmp_arg_cmp {
         match self.op {
             SeccompCmpOp::Eq => {
-                if syscall == ioctl {
-                    scmp_arg_cmp {
+                // When using EQ libseccomp compares the whole 64 bits. In
+                // general this is not a problem, but for example we have
+                // observed musl `ioctl` to leave garbage in the upper bits of
+                // the `request` argument. There is a GH issue to allow 32bit
+                // comparisons (see
+                // https://github.com/seccomp/libseccomp/issues/383) but is not
+                // merged yet. Until that is available, do a masked comparison
+                // with the upper 32bits set to 0, so we will compare that `hi32
+                // & 0x0 == 0`, which is always true. This costs one additional
+                // instruction, but will be likely be optimized away by the BPF
+                // JIT.
+                match self.val_len {
+                    SeccompCmpArgLen::Dword => scmp_arg_cmp {
                         arg: self.index as u32,
                         op: scmp_compare::SCMP_CMP_MASKED_EQ,
                         datum_a: 0x00000000FFFFFFFF,
                         datum_b: self.val,
-                    }
-                } else {
-                    scmp_arg_cmp {
+                    },
+                    SeccompCmpArgLen::Qword => scmp_arg_cmp {
                         arg: self.index as u32,
                         op: scmp_compare::SCMP_CMP_EQ,
                         datum_a: self.val,
                         datum_b: 0,
-                    }
+                    },
                 }
             }
             SeccompCmpOp::Ge => scmp_arg_cmp {