bpf_dsl: support arbitrary (arg & mask) == val expressions

Rework the seccomp_bpf compiler internals to work in terms of a single
general masked-equality condition instead of the variety of limited
condition operators previously supported.  All of the peephole
optimizations previously applied continue to be supported so similar
instructions should be emitted, but the handling of upper/lower words
is more cleanly structured now.

The old sandbox->Cond() interface continues to be supported for now so
that the old seccomp_bpf_unittests continue to give us assurances that
the new code generator is still correct.  Meanwhile, we provide a new
lower-level sandbox->CondMaskedEqual() method that bpf_dsl can now use.

BUG=408845
R=jln@chromium.org

Review URL: https://codereview.chromium.org/530133003

Cr-Commit-Position: refs/heads/master@{#293347}

sandbox/linux/bpf_dsl/bpf_dsl.cc

@@ -94,14 +94,15 @@ class PrimitiveBoolExprImpl : public internal::BoolExprImpl {
  public:
   PrimitiveBoolExprImpl(int argno,
                         ErrorCode::ArgType is_32bit,
-                        ErrorCode::Operation op,
+                        uint64_t mask,
                         uint64_t value)
-      : argno_(argno), is_32bit_(is_32bit), op_(op), value_(value) {}
+      : argno_(argno), is_32bit_(is_32bit), mask_(mask), value_(value) {}
 
   virtual ErrorCode Compile(SandboxBPF* sb,
                             ErrorCode true_ec,
                             ErrorCode false_ec) const OVERRIDE {
-    return sb->Cond(argno_, is_32bit_, op_, value_, true_ec, false_ec);
+    return sb->CondMaskedEqual(
+        argno_, is_32bit_, mask_, value_, true_ec, false_ec);
   }
 
  private:
@@ -109,7 +110,7 @@ class PrimitiveBoolExprImpl : public internal::BoolExprImpl {
 
   int argno_;
   ErrorCode::ArgType is_32bit_;
-  ErrorCode::Operation op_;
+  uint64_t mask_;
   uint64_t value_;
 
   DISALLOW_COPY_AND_ASSIGN(PrimitiveBoolExprImpl);
@@ -177,37 +178,26 @@ class OrBoolExprImpl : public internal::BoolExprImpl {
 
 namespace internal {
 
+uint64_t DefaultMask(size_t size) {
+  switch (size) {
+    case 4:
+      return std::numeric_limits<uint32_t>::max();
+    case 8:
+      return std::numeric_limits<uint64_t>::max();
+    default:
+      CHECK(false) << "Unimplemented DefaultMask case";
+      return 0;
+  }
+}
+
 BoolExpr ArgEq(int num, size_t size, uint64_t mask, uint64_t val) {
-  CHECK(num >= 0 && num < 6);
-  CHECK(size >= 1 && size <= 8);
-  CHECK_NE(0U, mask) << "zero mask doesn't make sense";
-  CHECK_EQ(val, val & mask) << "val contains masked out bits";
+  CHECK(size == 4 || size == 8);
 
   // TODO(mdempsky): Should we just always use TP_64BIT?
   const ErrorCode::ArgType arg_type =
-      (size <= 4) ? ErrorCode::TP_32BIT : ErrorCode::TP_64BIT;
-
-  if (mask == std::numeric_limits<uint64_t>::max()) {
-    // Arg == Val
-    return BoolExpr(new const PrimitiveBoolExprImpl(
-        num, arg_type, ErrorCode::OP_EQUAL, val));
-  }
-
-  if (mask == val) {
-    // (Arg & Mask) == Mask
-    return BoolExpr(new const PrimitiveBoolExprImpl(
-        num, arg_type, ErrorCode::OP_HAS_ALL_BITS, mask));
-  }
-
-  if (val == 0) {
-    // (Arg & Mask) == 0, which is semantically equivalent to !((arg & mask) !=
-    // 0).
-    return !BoolExpr(new const PrimitiveBoolExprImpl(
-        num, arg_type, ErrorCode::OP_HAS_ANY_BITS, mask));
-  }
+      (size == 4) ? ErrorCode::TP_32BIT : ErrorCode::TP_64BIT;
 
-  CHECK(false) << "Unimplemented ArgEq case";
-  return BoolExpr();
+  return BoolExpr(new const PrimitiveBoolExprImpl(num, arg_type, mask, val));
 }
 
 }  // namespace internal

sandbox/linux/bpf_dsl/bpf_dsl.h

@@ -7,7 +7,6 @@
 
 #include <stdint.h>
 
-#include <limits>
 #include <utility>
 
 #include "base/macros.h"
@@ -62,7 +61,7 @@ class SandboxBPF;
 //
 //   result = Allow() | Error(errno) | Trap(trap_func, arg)
 //          | If(bool, result)[.ElseIf(bool, result)].Else(result)
-//   bool   = arg == val | (arg & mask) == mask | (arg & mask) == 0
+//   bool   = arg == val | (arg & mask) == val
 //          | !bool | bool && bool | bool || bool
 //
 // The semantics of each function and operator are intended to be
@@ -135,8 +134,7 @@ class SANDBOX_EXPORT Arg {
  public:
   // Initializes the Arg to represent the |num|th system call
   // argument (indexed from 0), which is of type |T|.
-  explicit Arg(int num)
-      : num_(num), mask_(std::numeric_limits<uint64_t>::max()) {}
+  explicit Arg(int num);
 
   Arg(const Arg& arg) : num_(arg.num_), mask_(arg.mask_) {}
 
@@ -211,6 +209,9 @@ namespace internal {
 SANDBOX_EXPORT BoolExpr
     ArgEq(int num, size_t size, uint64_t mask, uint64_t val);
 
+// Returns the default mask for a system call argument of the specified size.
+SANDBOX_EXPORT uint64_t DefaultMask(size_t size);
+
 // Internal interface implemented by BoolExpr implementations.
 class SANDBOX_EXPORT BoolExprImpl : public base::RefCounted<BoolExprImpl> {
  public:
@@ -243,6 +244,11 @@ class SANDBOX_EXPORT ResultExprImpl : public base::RefCounted<ResultExprImpl> {
 
 }  // namespace internal
 
+template <typename T>
+Arg<T>::Arg(int num)
+    : num_(num), mask_(internal::DefaultMask(sizeof(T))) {
+}
+
 // Definition requires ArgEq to have been declared.  Moved out-of-line
 // to minimize how much internal clutter users have to ignore while
 // reading the header documentation.

sandbox/linux/bpf_dsl/bpf_dsl_unittest.cc

@@ -32,6 +32,9 @@ class Stubs {
   static int getpgid(pid_t pid) { return Syscall::Call(__NR_getpgid, pid); }
   static int setuid(uid_t uid) { return Syscall::Call(__NR_setuid, uid); }
   static int setgid(gid_t gid) { return Syscall::Call(__NR_setgid, gid); }
+  static int setpgid(pid_t pid, pid_t pgid) {
+    return Syscall::Call(__NR_setpgid, pid, pgid);
+  }
 
   static int uname(struct utsname* buf) {
     return Syscall::Call(__NR_uname, buf);
@@ -212,6 +215,10 @@ class MaskingPolicy : public SandboxBPFDSLPolicy {
       const Arg<gid_t> gid(0);
       return If((gid & 0xf0) == 0xf0, Error(EINVAL)).Else(Error(EACCES));
     }
+    if (sysno == __NR_setpgid) {
+      const Arg<pid_t> pid(0);
+      return If((pid & 0xa5) == 0xa0, Error(EINVAL)).Else(Error(EACCES));
+    }
     return Allow();
   }
 
@@ -229,6 +236,11 @@ BPF_TEST_C(BPFDSL, MaskTest, MaskingPolicy) {
     const int expect_errno = (gid & 0xf0) == 0xf0 ? EINVAL : EACCES;
     ASSERT_SYSCALL_RESULT(-expect_errno, setgid, gid);
   }
+
+  for (pid_t pid = 0; pid < 0x100; ++pid) {
+    const int expect_errno = (pid & 0xa5) == 0xa0 ? EINVAL : EACCES;
+    ASSERT_SYSCALL_RESULT(-expect_errno, setpgid, pid, 0);
+  }
 }
 
 class ElseIfPolicy : public SandboxBPFDSLPolicy {

sandbox/linux/seccomp-bpf/errorcode.cc

@@ -36,21 +36,18 @@ ErrorCode::ErrorCode(Trap::TrapFnc fnc, const void* aux, bool safe, uint16_t id)
 
 ErrorCode::ErrorCode(int argno,
                      ArgType width,
-                     Operation op,
+                     uint64_t mask,
                      uint64_t value,
                      const ErrorCode* passed,
                      const ErrorCode* failed)
     : error_type_(ET_COND),
+      mask_(mask),
       value_(value),
       argno_(argno),
       width_(width),
-      op_(op),
       passed_(passed),
       failed_(failed),
       err_(SECCOMP_RET_INVALID) {
-  if (op < 0 || op >= OP_NUM_OPS) {
-    SANDBOX_DIE("Invalid opcode in BPF sandbox rules");
-  }
 }
 
 bool ErrorCode::Equals(const ErrorCode& err) const {
@@ -63,9 +60,9 @@ bool ErrorCode::Equals(const ErrorCode& err) const {
   if (error_type_ == ET_SIMPLE || error_type_ == ET_TRAP) {
     return err_ == err.err_;
   } else if (error_type_ == ET_COND) {
-    return value_ == err.value_ && argno_ == err.argno_ &&
-           width_ == err.width_ && op_ == err.op_ &&
-           passed_->Equals(*err.passed_) && failed_->Equals(*err.failed_);
+    return mask_ == err.mask_ && value_ == err.value_ && argno_ == err.argno_ &&
+           width_ == err.width_ && passed_->Equals(*err.passed_) &&
+           failed_->Equals(*err.failed_);
   } else {
     SANDBOX_DIE("Corrupted ErrorCode");
   }
@@ -85,14 +82,14 @@ bool ErrorCode::LessThan(const ErrorCode& err) const {
     if (error_type_ == ET_SIMPLE || error_type_ == ET_TRAP) {
       return err_ < err.err_;
     } else if (error_type_ == ET_COND) {
-      if (value_ != err.value_) {
+      if (mask_ != err.mask_) {
+        return mask_ < err.mask_;
+      } else if (value_ != err.value_) {
         return value_ < err.value_;
       } else if (argno_ != err.argno_) {
         return argno_ < err.argno_;
       } else if (width_ != err.width_) {
         return width_ < err.width_;
-      } else if (op_ != err.op_) {
-        return op_ < err.op_;
       } else if (!passed_->Equals(*err.passed_)) {
         return passed_->LessThan(*err.passed_);
       } else if (!failed_->Equals(*err.failed_)) {

sandbox/linux/seccomp-bpf/errorcode.h

@@ -87,19 +87,11 @@ class SANDBOX_EXPORT ErrorCode {
     TP_64BIT,
   };
 
+  // Deprecated.
   enum Operation {
     // Test whether the system call argument is equal to the operand.
     OP_EQUAL,
 
-    // Test whether the system call argument is greater (or equal) to the
-    // operand. Please note that all tests always operate on unsigned
-    // values. You can generally emulate signed tests, if that's what you
-    // need.
-    // TODO(markus): Check whether we should automatically emulate signed
-    //               operations.
-    OP_GREATER_UNSIGNED,
-    OP_GREATER_EQUAL_UNSIGNED,
-
     // Tests a system call argument against a bit mask.
     // The "ALL_BITS" variant performs this test: "arg & mask == mask"
     // This implies that a mask of zero always results in a passing test.
@@ -107,9 +99,6 @@ class SANDBOX_EXPORT ErrorCode {
     // This implies that a mask of zero always results in a failing test.
     OP_HAS_ALL_BITS,
     OP_HAS_ANY_BITS,
-
-    // Total number of operations.
-    OP_NUM_OPS,
   };
 
   enum ErrorType {
@@ -145,10 +134,10 @@ class SANDBOX_EXPORT ErrorCode {
 
   bool safe() const { return safe_; }
 
+  uint64_t mask() const { return mask_; }
   uint64_t value() const { return value_; }
   int argno() const { return argno_; }
   ArgType width() const { return width_; }
-  Operation op() const { return op_; }
   const ErrorCode* passed() const { return passed_; }
   const ErrorCode* failed() const { return failed_; }
 
@@ -172,7 +161,7 @@ class SANDBOX_EXPORT ErrorCode {
   // allows us to specify additional constraints.
   ErrorCode(int argno,
             ArgType width,
-            Operation op,
+            uint64_t mask,
             uint64_t value,
             const ErrorCode* passed,
             const ErrorCode* failed);
@@ -189,10 +178,10 @@ class SANDBOX_EXPORT ErrorCode {
 
     // Fields needed when inspecting additional arguments.
     struct {
+      uint64_t mask_;            // Mask that we are comparing under.
       uint64_t value_;           // Value that we are comparing with.
       int argno_;                // Syscall arg number that we are inspecting.
       ArgType width_;            // Whether we are looking at a 32/64bit value.
-      Operation op_;             // Comparison operation.
       const ErrorCode* passed_;  // Value to be returned if comparison passed,
       const ErrorCode* failed_;  //   or if it failed.
     };

sandbox/linux/seccomp-bpf/sandbox_bpf.h

@@ -147,13 +147,21 @@ class SANDBOX_EXPORT SandboxBPF {
   // We can also use ErrorCode to request evaluation of a conditional
   // statement based on inspection of system call parameters.
   // This method wrap an ErrorCode object around the conditional statement.
-  // Argument "argno" (1..6) will be compared to "value" using comparator
-  // "op". If the condition is true "passed" will be returned, otherwise
-  // "failed".
+  // Argument "argno" (1..6) will be bitwise-AND'd with "mask" and compared
+  // to "value"; if equal, then "passed" will be returned, otherwise "failed".
   // If "is32bit" is set, the argument must in the range of 0x0..(1u << 32 - 1)
   // If it is outside this range, the sandbox treats the system call just
   // the same as any other ABI violation (i.e. it aborts with an error
   // message).
+  ErrorCode CondMaskedEqual(int argno,
+                            ErrorCode::ArgType is_32bit,
+                            uint64_t mask,
+                            uint64_t value,
+                            const ErrorCode& passed,
+                            const ErrorCode& failed);
+
+  // Legacy variant of CondMaskedEqual that supports a few comparison
+  // operations, which get converted into masked-equality comparisons.
   ErrorCode Cond(int argno,
                  ErrorCode::ArgType is_32bit,
                  ErrorCode::Operation op,
@@ -209,6 +217,13 @@ class SANDBOX_EXPORT SandboxBPF {
   typedef std::map<uint32_t, ErrorCode> ErrMap;
   typedef std::set<ErrorCode, struct ErrorCode::LessThan> Conds;
 
+  // Used by CondExpressionHalf to track which half of the argument it's
+  // emitting instructions for.
+  enum ArgHalf {
+    LowerHalf,
+    UpperHalf,
+  };
+
   // Get a file descriptor pointing to "/proc", if currently available.
   int proc_fd() { return proc_fd_; }
 
@@ -261,6 +276,14 @@ class SANDBOX_EXPORT SandboxBPF {
   // called from RetExpression().
   Instruction* CondExpression(CodeGen* gen, const ErrorCode& cond);
 
+  // Returns a BPF program that evaluates half of a conditional expression;
+  // it should only ever be called from CondExpression().
+  Instruction* CondExpressionHalf(CodeGen* gen,
+                                  const ErrorCode& cond,
+                                  ArgHalf half,
+                                  Instruction* passed,
+                                  Instruction* failed);
+
   static SandboxStatus status_;
 
   bool quiet_;

sandbox/linux/seccomp-bpf/sandbox_bpf_unittest.cc

@@ -1733,6 +1733,115 @@ BPF_TEST_C(SandboxBPF, AnyBitTests, AnyBitTestPolicy) {
   BITMASK_TEST( 10,                -1L, ANYBITS64,0x100000001, EXPECT_SUCCESS);
 }
 
+class MaskedEqualTestPolicy : public SandboxBPFPolicy {
+ public:
+  MaskedEqualTestPolicy() {}
+  virtual ErrorCode EvaluateSyscall(SandboxBPF* sandbox,
+                                    int sysno) const OVERRIDE;
+
+ private:
+  struct Rule {
+    ErrorCode::ArgType arg_type;
+    uint64_t mask;
+    uint64_t value;
+  };
+
+  static Rule rules[];
+
+  DISALLOW_COPY_AND_ASSIGN(MaskedEqualTestPolicy);
+};
+
+MaskedEqualTestPolicy::Rule MaskedEqualTestPolicy::rules[] = {
+    /* 0 = */ {ErrorCode::TP_32BIT, 0x0000000000ff00ff, 0x00000000005500aa},
+
+#if __SIZEOF_POINTER__ > 4
+    /* 1 = */ {ErrorCode::TP_64BIT, 0x00ff00ff00000000, 0x005500aa00000000},
+    /* 2 = */ {ErrorCode::TP_64BIT, 0x00ff00ff00ff00ff, 0x005500aa005500aa},
+#endif
+};
+
+ErrorCode MaskedEqualTestPolicy::EvaluateSyscall(SandboxBPF* sandbox,
+                                                 int sysno) const {
+  DCHECK(SandboxBPF::IsValidSyscallNumber(sysno));
+
+  if (sysno == __NR_uname) {
+    ErrorCode err = sandbox->Kill("Invalid test case number");
+    for (size_t i = 0; i < arraysize(rules); i++) {
+      err = sandbox->Cond(0,
+                          ErrorCode::TP_32BIT,
+                          ErrorCode::OP_EQUAL,
+                          i,
+                          sandbox->CondMaskedEqual(1,
+                                                   rules[i].arg_type,
+                                                   rules[i].mask,
+                                                   rules[i].value,
+                                                   ErrorCode(1),
+                                                   ErrorCode(0)),
+                          err);
+    }
+    return err;
+  }
+  return ErrorCode(ErrorCode::ERR_ALLOWED);
+}
+
+#define MASKEQ_TEST(rulenum, arg, expected_result) \
+  BPF_ASSERT(Syscall::Call(__NR_uname, (rulenum), (arg)) == (expected_result))
+
+BPF_TEST_C(SandboxBPF, MaskedEqualTests, MaskedEqualTestPolicy) {
+  // Allowed:    0x__55__aa
+  MASKEQ_TEST(0, 0x00000000, EXPECT_FAILURE);
+  MASKEQ_TEST(0, 0x00000001, EXPECT_FAILURE);
+  MASKEQ_TEST(0, 0x00000003, EXPECT_FAILURE);
+  MASKEQ_TEST(0, 0x00000100, EXPECT_FAILURE);
+  MASKEQ_TEST(0, 0x00000300, EXPECT_FAILURE);
+  MASKEQ_TEST(0, 0x005500aa, EXPECT_SUCCESS);
+  MASKEQ_TEST(0, 0x005500ab, EXPECT_FAILURE);
+  MASKEQ_TEST(0, 0x005600aa, EXPECT_FAILURE);
+  MASKEQ_TEST(0, 0x005501aa, EXPECT_SUCCESS);
+  MASKEQ_TEST(0, 0x005503aa, EXPECT_SUCCESS);
+  MASKEQ_TEST(0, 0x555500aa, EXPECT_SUCCESS);
+  MASKEQ_TEST(0, 0xaa5500aa, EXPECT_SUCCESS);
+
+#if __SIZEOF_POINTER__ > 4
+  // Allowed:    0x__55__aa________
+  MASKEQ_TEST(1, 0x0000000000000000, EXPECT_FAILURE);
+  MASKEQ_TEST(1, 0x0000000000000010, EXPECT_FAILURE);
+  MASKEQ_TEST(1, 0x0000000000000050, EXPECT_FAILURE);
+  MASKEQ_TEST(1, 0x0000000100000000, EXPECT_FAILURE);
+  MASKEQ_TEST(1, 0x0000000300000000, EXPECT_FAILURE);
+  MASKEQ_TEST(1, 0x0000010000000000, EXPECT_FAILURE);
+  MASKEQ_TEST(1, 0x0000030000000000, EXPECT_FAILURE);
+  MASKEQ_TEST(1, 0x005500aa00000000, EXPECT_SUCCESS);
+  MASKEQ_TEST(1, 0x005500ab00000000, EXPECT_FAILURE);
+  MASKEQ_TEST(1, 0x005600aa00000000, EXPECT_FAILURE);
+  MASKEQ_TEST(1, 0x005501aa00000000, EXPECT_SUCCESS);
+  MASKEQ_TEST(1, 0x005503aa00000000, EXPECT_SUCCESS);
+  MASKEQ_TEST(1, 0x555500aa00000000, EXPECT_SUCCESS);
+  MASKEQ_TEST(1, 0xaa5500aa00000000, EXPECT_SUCCESS);
+  MASKEQ_TEST(1, 0xaa5500aa00000000, EXPECT_SUCCESS);
+  MASKEQ_TEST(1, 0xaa5500aa0000cafe, EXPECT_SUCCESS);
+
+  // Allowed:    0x__55__aa__55__aa
+  MASKEQ_TEST(2, 0x0000000000000000, EXPECT_FAILURE);
+  MASKEQ_TEST(2, 0x0000000000000010, EXPECT_FAILURE);
+  MASKEQ_TEST(2, 0x0000000000000050, EXPECT_FAILURE);
+  MASKEQ_TEST(2, 0x0000000100000000, EXPECT_FAILURE);
+  MASKEQ_TEST(2, 0x0000000300000000, EXPECT_FAILURE);
+  MASKEQ_TEST(2, 0x0000010000000000, EXPECT_FAILURE);
+  MASKEQ_TEST(2, 0x0000030000000000, EXPECT_FAILURE);
+  MASKEQ_TEST(2, 0x00000000005500aa, EXPECT_FAILURE);
+  MASKEQ_TEST(2, 0x005500aa00000000, EXPECT_FAILURE);
+  MASKEQ_TEST(2, 0x005500aa005500aa, EXPECT_SUCCESS);
+  MASKEQ_TEST(2, 0x005500aa005700aa, EXPECT_FAILURE);
+  MASKEQ_TEST(2, 0x005700aa005500aa, EXPECT_FAILURE);
+  MASKEQ_TEST(2, 0x005500aa004500aa, EXPECT_FAILURE);
+  MASKEQ_TEST(2, 0x004500aa005500aa, EXPECT_FAILURE);
+  MASKEQ_TEST(2, 0x005512aa005500aa, EXPECT_SUCCESS);
+  MASKEQ_TEST(2, 0x005500aa005534aa, EXPECT_SUCCESS);
+  MASKEQ_TEST(2, 0xff5500aa0055ffaa, EXPECT_SUCCESS);
+#endif
+}
+
 intptr_t PthreadTrapHandler(const struct arch_seccomp_data& args, void* aux) {
   if (args.args[0] != (CLONE_CHILD_CLEARTID | CLONE_CHILD_SETTID | SIGCHLD)) {
     // We expect to get called for an attempt to fork(). No need to log that