From 1f9ec0e417465d285e5033417a8b9456f3140a68 Mon Sep 17 00:00:00 2001 From: mdempsky Date: Fri, 4 Sep 2015 12:32:52 -0700 Subject: [PATCH] sandbox/linux/bpf_dsl: remove ErrorCode intermediary representation This changes PolicyCompiler to skip the pass that converts ResultExprs into ErrorCodes, and instead it directly emits instructions using CodeGen. Reduces the size of sandbox_linux_unittests in release builds on amd64 by about 30kB. BUG=414363 Review URL: https://codereview.chromium.org/1309963002 Cr-Commit-Position: refs/heads/master@{#347470} --- sandbox/linux/BUILD.gn | 2 - sandbox/linux/bpf_dsl/bpf_dsl.cc | 90 +++++---- sandbox/linux/bpf_dsl/bpf_dsl_impl.h | 17 +- sandbox/linux/bpf_dsl/errorcode.cc | 120 ------------ sandbox/linux/bpf_dsl/errorcode.h | 176 +----------------- sandbox/linux/bpf_dsl/errorcode_unittest.cc | 120 ------------ sandbox/linux/bpf_dsl/policy_compiler.cc | 126 +++++-------- sandbox/linux/bpf_dsl/policy_compiler.h | 86 ++++----- sandbox/linux/sandbox_linux.gypi | 1 - sandbox/linux/sandbox_linux_nacl_nonsfi.gyp | 1 - sandbox/linux/sandbox_linux_test_sources.gypi | 1 - 11 files changed, 141 insertions(+), 599 deletions(-) delete mode 100644 sandbox/linux/bpf_dsl/errorcode.cc delete mode 100644 sandbox/linux/bpf_dsl/errorcode_unittest.cc diff --git a/sandbox/linux/BUILD.gn b/sandbox/linux/BUILD.gn index d693df6708929b..566735ac59883b 100644 --- a/sandbox/linux/BUILD.gn +++ b/sandbox/linux/BUILD.gn @@ -119,7 +119,6 @@ source_set("sandbox_linux_unittests_sources") { "bpf_dsl/cons_unittest.cc", "bpf_dsl/dump_bpf.cc", "bpf_dsl/dump_bpf.h", - "bpf_dsl/errorcode_unittest.cc", "bpf_dsl/syscall_set_unittest.cc", "bpf_dsl/test_trap_registry.cc", "bpf_dsl/test_trap_registry.h", @@ -218,7 +217,6 @@ component("seccomp_bpf") { "bpf_dsl/codegen.cc", "bpf_dsl/codegen.h", "bpf_dsl/cons.h", - "bpf_dsl/errorcode.cc", "bpf_dsl/errorcode.h", "bpf_dsl/linux_syscall_ranges.h", "bpf_dsl/policy.cc", diff --git a/sandbox/linux/bpf_dsl/bpf_dsl.cc b/sandbox/linux/bpf_dsl/bpf_dsl.cc index 276aaf6e04f28f..0363be068e7253 100644 --- a/sandbox/linux/bpf_dsl/bpf_dsl.cc +++ b/sandbox/linux/bpf_dsl/bpf_dsl.cc @@ -11,6 +11,7 @@ #include "sandbox/linux/bpf_dsl/bpf_dsl_impl.h" #include "sandbox/linux/bpf_dsl/errorcode.h" #include "sandbox/linux/bpf_dsl/policy_compiler.h" +#include "sandbox/linux/system_headers/linux_seccomp.h" namespace sandbox { namespace bpf_dsl { @@ -20,8 +21,8 @@ class AllowResultExprImpl : public internal::ResultExprImpl { public: AllowResultExprImpl() {} - ErrorCode Compile(PolicyCompiler* pc) const override { - return ErrorCode(ErrorCode::ERR_ALLOWED); + CodeGen::Node Compile(PolicyCompiler* pc) const override { + return pc->Return(SECCOMP_RET_ALLOW); } bool IsAllow() const override { return true; } @@ -38,8 +39,8 @@ class ErrorResultExprImpl : public internal::ResultExprImpl { CHECK(err_ >= ErrorCode::ERR_MIN_ERRNO && err_ <= ErrorCode::ERR_MAX_ERRNO); } - ErrorCode Compile(PolicyCompiler* pc) const override { - return pc->Error(err_); + CodeGen::Node Compile(PolicyCompiler* pc) const override { + return pc->Return(SECCOMP_RET_ERRNO + err_); } bool IsDeny() const override { return true; } @@ -56,8 +57,8 @@ class KillResultExprImpl : public internal::ResultExprImpl { public: KillResultExprImpl() {} - ErrorCode Compile(PolicyCompiler* pc) const override { - return ErrorCode(ErrorCode::ERR_KILL); + CodeGen::Node Compile(PolicyCompiler* pc) const override { + return pc->Return(SECCOMP_RET_KILL); } bool IsDeny() const override { return true; } @@ -72,8 +73,8 @@ class TraceResultExprImpl : public internal::ResultExprImpl { public: TraceResultExprImpl(uint16_t aux) : aux_(aux) {} - ErrorCode Compile(PolicyCompiler* pc) const override { - return ErrorCode(ErrorCode::ERR_TRACE + aux_); + CodeGen::Node Compile(PolicyCompiler* pc) const override { + return pc->Return(SECCOMP_RET_TRACE + aux_); } private: @@ -91,7 +92,7 @@ class TrapResultExprImpl : public internal::ResultExprImpl { DCHECK(func_); } - ErrorCode Compile(PolicyCompiler* pc) const override { + CodeGen::Node Compile(PolicyCompiler* pc) const override { return pc->Trap(func_, arg_, safe_); } @@ -116,7 +117,7 @@ class IfThenResultExprImpl : public internal::ResultExprImpl { const ResultExpr& else_result) : cond_(cond), then_result_(then_result), else_result_(else_result) {} - ErrorCode Compile(PolicyCompiler* pc) const override { + CodeGen::Node Compile(PolicyCompiler* pc) const override { return cond_->Compile( pc, then_result_->Compile(pc), else_result_->Compile(pc)); } @@ -139,10 +140,10 @@ class ConstBoolExprImpl : public internal::BoolExprImpl { public: ConstBoolExprImpl(bool value) : value_(value) {} - ErrorCode Compile(PolicyCompiler* pc, - ErrorCode true_ec, - ErrorCode false_ec) const override { - return value_ ? true_ec : false_ec; + CodeGen::Node Compile(PolicyCompiler* pc, + CodeGen::Node then_node, + CodeGen::Node else_node) const override { + return value_ ? then_node : else_node; } private: @@ -153,40 +154,39 @@ class ConstBoolExprImpl : public internal::BoolExprImpl { DISALLOW_COPY_AND_ASSIGN(ConstBoolExprImpl); }; -class PrimitiveBoolExprImpl : public internal::BoolExprImpl { +class MaskedEqualBoolExprImpl : public internal::BoolExprImpl { public: - PrimitiveBoolExprImpl(int argno, - ErrorCode::ArgType is_32bit, - uint64_t mask, - uint64_t value) - : argno_(argno), is_32bit_(is_32bit), mask_(mask), value_(value) {} - - ErrorCode Compile(PolicyCompiler* pc, - ErrorCode true_ec, - ErrorCode false_ec) const override { - return pc->CondMaskedEqual( - argno_, is_32bit_, mask_, value_, true_ec, false_ec); + MaskedEqualBoolExprImpl(int argno, + size_t width, + uint64_t mask, + uint64_t value) + : argno_(argno), width_(width), mask_(mask), value_(value) {} + + CodeGen::Node Compile(PolicyCompiler* pc, + CodeGen::Node then_node, + CodeGen::Node else_node) const override { + return pc->MaskedEqual(argno_, width_, mask_, value_, then_node, else_node); } private: - ~PrimitiveBoolExprImpl() override {} + ~MaskedEqualBoolExprImpl() override {} int argno_; - ErrorCode::ArgType is_32bit_; + size_t width_; uint64_t mask_; uint64_t value_; - DISALLOW_COPY_AND_ASSIGN(PrimitiveBoolExprImpl); + DISALLOW_COPY_AND_ASSIGN(MaskedEqualBoolExprImpl); }; class NegateBoolExprImpl : public internal::BoolExprImpl { public: explicit NegateBoolExprImpl(const BoolExpr& cond) : cond_(cond) {} - ErrorCode Compile(PolicyCompiler* pc, - ErrorCode true_ec, - ErrorCode false_ec) const override { - return cond_->Compile(pc, false_ec, true_ec); + CodeGen::Node Compile(PolicyCompiler* pc, + CodeGen::Node then_node, + CodeGen::Node else_node) const override { + return cond_->Compile(pc, else_node, then_node); } private: @@ -202,10 +202,11 @@ class AndBoolExprImpl : public internal::BoolExprImpl { AndBoolExprImpl(const BoolExpr& lhs, const BoolExpr& rhs) : lhs_(lhs), rhs_(rhs) {} - ErrorCode Compile(PolicyCompiler* pc, - ErrorCode true_ec, - ErrorCode false_ec) const override { - return lhs_->Compile(pc, rhs_->Compile(pc, true_ec, false_ec), false_ec); + CodeGen::Node Compile(PolicyCompiler* pc, + CodeGen::Node then_node, + CodeGen::Node else_node) const override { + return lhs_->Compile(pc, rhs_->Compile(pc, then_node, else_node), + else_node); } private: @@ -222,10 +223,11 @@ class OrBoolExprImpl : public internal::BoolExprImpl { OrBoolExprImpl(const BoolExpr& lhs, const BoolExpr& rhs) : lhs_(lhs), rhs_(rhs) {} - ErrorCode Compile(PolicyCompiler* pc, - ErrorCode true_ec, - ErrorCode false_ec) const override { - return lhs_->Compile(pc, true_ec, rhs_->Compile(pc, true_ec, false_ec)); + CodeGen::Node Compile(PolicyCompiler* pc, + CodeGen::Node then_node, + CodeGen::Node else_node) const override { + return lhs_->Compile(pc, then_node, + rhs_->Compile(pc, then_node, else_node)); } private: @@ -270,11 +272,7 @@ BoolExpr ArgEq(int num, size_t size, uint64_t mask, uint64_t val) { // accordingly. 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; - - return BoolExpr(new const PrimitiveBoolExprImpl(num, arg_type, mask, val)); + return BoolExpr(new const MaskedEqualBoolExprImpl(num, size, mask, val)); } } // namespace internal diff --git a/sandbox/linux/bpf_dsl/bpf_dsl_impl.h b/sandbox/linux/bpf_dsl/bpf_dsl_impl.h index 709a31c5da3cd8..0064f8a7a2176f 100644 --- a/sandbox/linux/bpf_dsl/bpf_dsl_impl.h +++ b/sandbox/linux/bpf_dsl/bpf_dsl_impl.h @@ -7,6 +7,7 @@ #include "base/macros.h" #include "base/memory/ref_counted.h" +#include "sandbox/linux/bpf_dsl/codegen.h" #include "sandbox/sandbox_export.h" namespace sandbox { @@ -19,12 +20,12 @@ namespace internal { // Internal interface implemented by BoolExpr implementations. class BoolExprImpl : public base::RefCounted { public: - // Compile uses |pc| to construct an ErrorCode that conditionally continues - // to either |true_ec| or |false_ec|, depending on whether the represented + // Compile uses |pc| to emit a CodeGen::Node that conditionally continues + // to either |then_node| or |false_node|, depending on whether the represented // boolean expression is true or false. - virtual ErrorCode Compile(PolicyCompiler* pc, - ErrorCode true_ec, - ErrorCode false_ec) const = 0; + virtual CodeGen::Node Compile(PolicyCompiler* pc, + CodeGen::Node then_node, + CodeGen::Node else_node) const = 0; protected: BoolExprImpl() {} @@ -38,9 +39,9 @@ class BoolExprImpl : public base::RefCounted { // Internal interface implemented by ResultExpr implementations. class ResultExprImpl : public base::RefCounted { public: - // Compile uses |pc| to construct an ErrorCode analogous to the represented - // result expression. - virtual ErrorCode Compile(PolicyCompiler* pc) const = 0; + // Compile uses |pc| to emit a CodeGen::Node that executes the + // represented result expression. + virtual CodeGen::Node Compile(PolicyCompiler* pc) const = 0; // HasUnsafeTraps returns whether the result expression is or recursively // contains an unsafe trap expression. diff --git a/sandbox/linux/bpf_dsl/errorcode.cc b/sandbox/linux/bpf_dsl/errorcode.cc deleted file mode 100644 index 8660f714418955..00000000000000 --- a/sandbox/linux/bpf_dsl/errorcode.cc +++ /dev/null @@ -1,120 +0,0 @@ -// Copyright (c) 2012 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "sandbox/linux/bpf_dsl/errorcode.h" - -#include "base/logging.h" -#include "sandbox/linux/system_headers/linux_seccomp.h" - -namespace sandbox { -namespace bpf_dsl { - -ErrorCode::ErrorCode() : error_type_(ET_INVALID), err_(SECCOMP_RET_INVALID) {} - -ErrorCode::ErrorCode(int err) { - switch (err) { - case ERR_ALLOWED: - err_ = SECCOMP_RET_ALLOW; - error_type_ = ET_SIMPLE; - break; - case ERR_KILL: - err_ = SECCOMP_RET_KILL; - error_type_ = ET_SIMPLE; - break; - case ERR_MIN_ERRNO... ERR_MAX_ERRNO: - err_ = SECCOMP_RET_ERRNO + err; - error_type_ = ET_SIMPLE; - break; - default: - if ((err & ~SECCOMP_RET_DATA) == ERR_TRACE) { - err_ = SECCOMP_RET_TRACE + (err & SECCOMP_RET_DATA); - error_type_ = ET_SIMPLE; - break; - } - LOG(FATAL) << "Invalid use of ErrorCode object"; - } -} - -ErrorCode::ErrorCode(uint16_t trap_id, - TrapRegistry::TrapFnc fnc, - const void* aux, - bool safe) - : error_type_(ET_TRAP), - fnc_(fnc), - aux_(const_cast(aux)), - safe_(safe), - err_(SECCOMP_RET_TRAP + trap_id) {} - -ErrorCode::ErrorCode(int argno, - ArgType width, - uint64_t mask, - uint64_t value, - const ErrorCode* passed, - const ErrorCode* failed) - : error_type_(ET_COND), - mask_(mask), - value_(value), - argno_(argno), - width_(width), - passed_(passed), - failed_(failed), - err_(SECCOMP_RET_INVALID) {} - -bool ErrorCode::Equals(const ErrorCode& err) const { - if (error_type_ == ET_INVALID || err.error_type_ == ET_INVALID) { - LOG(FATAL) << "Dereferencing invalid ErrorCode"; - } - if (error_type_ != err.error_type_) { - return false; - } - if (error_type_ == ET_SIMPLE || error_type_ == ET_TRAP) { - return err_ == err.err_; - } else if (error_type_ == ET_COND) { - return mask_ == err.mask_ && value_ == err.value_ && argno_ == err.argno_ && - width_ == err.width_ && passed_->Equals(*err.passed_) && - failed_->Equals(*err.failed_); - } else { - LOG(FATAL) << "Corrupted ErrorCode"; - return false; - } -} - -bool ErrorCode::LessThan(const ErrorCode& err) const { - // Implementing a "LessThan()" operator allows us to use ErrorCode objects - // as keys in STL containers; most notably, it also allows us to put them - // into std::set<>. Actual ordering is not important as long as it is - // deterministic. - if (error_type_ == ET_INVALID || err.error_type_ == ET_INVALID) { - LOG(FATAL) << "Dereferencing invalid ErrorCode"; - } - if (error_type_ != err.error_type_) { - return error_type_ < err.error_type_; - } else { - if (error_type_ == ET_SIMPLE || error_type_ == ET_TRAP) { - return err_ < err.err_; - } else if (error_type_ == ET_COND) { - 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 (!passed_->Equals(*err.passed_)) { - return passed_->LessThan(*err.passed_); - } else if (!failed_->Equals(*err.failed_)) { - return failed_->LessThan(*err.failed_); - } else { - return false; - } - } else { - LOG(FATAL) << "Corrupted ErrorCode"; - return false; - } - } -} - -} // namespace bpf_dsl -} // namespace sandbox diff --git a/sandbox/linux/bpf_dsl/errorcode.h b/sandbox/linux/bpf_dsl/errorcode.h index 15f8339e51bdc5..611c27dd80712e 100644 --- a/sandbox/linux/bpf_dsl/errorcode.h +++ b/sandbox/linux/bpf_dsl/errorcode.h @@ -5,46 +5,17 @@ #ifndef SANDBOX_LINUX_BPF_DSL_ERRORCODE_H__ #define SANDBOX_LINUX_BPF_DSL_ERRORCODE_H__ -#include "sandbox/linux/bpf_dsl/trap_registry.h" +#include "base/macros.h" #include "sandbox/sandbox_export.h" namespace sandbox { namespace bpf_dsl { -// This class holds all the possible values that can be returned by a sandbox -// policy. -// We can either wrap a symbolic ErrorCode (i.e. ERR_XXX enum values), an -// errno value (in the range 0..4095), a pointer to a TrapFnc callback -// handling a SECCOMP_RET_TRAP trap, or a complex constraint. -// All of the commonly used values are stored in the "err_" field. So, code -// that is using the ErrorCode class typically operates on a single 32bit -// field. -// -// TODO(mdempsky): Nuke from orbit. The only reason this class still -// exists is for Verifier, which will eventually be replaced by a true -// BPF symbolic evaluator and constraint solver. +// TODO(mdempsky): Find a proper home for ERR_{MIN,MAX}_ERRNO and +// remove this header. class SANDBOX_EXPORT ErrorCode { public: enum { - // Allow this system call. The value of ERR_ALLOWED is pretty much - // completely arbitrary. But we want to pick it so that is is unlikely - // to be passed in accidentally, when the user intended to return an - // "errno" (see below) value instead. - ERR_ALLOWED = 0x04000000, - - // If the progress is being ptraced with PTRACE_O_TRACESECCOMP, then the - // tracer will be notified of a PTRACE_EVENT_SECCOMP and allowed to change - // or skip the system call. The lower 16 bits of err will be available to - // the tracer via PTRACE_GETEVENTMSG. - ERR_TRACE = 0x08000000, - - // Kill the process immediately. - ERR_KILL = 0x10000000, - - // Deny the system call with a particular "errno" value. - // N.B.: It is also possible to return "0" here. That would normally - // indicate success, but it won't actually run the system call. - // This is very different from return ERR_ALLOWED. ERR_MIN_ERRNO = 0, #if defined(__mips__) // MIPS only supports errno up to 1133 @@ -56,147 +27,8 @@ class SANDBOX_EXPORT ErrorCode { #endif }; - // While BPF filter programs always operate on 32bit quantities, the kernel - // always sees system call arguments as 64bit values. This statement is true - // no matter whether the host system is natively operating in 32bit or 64bit. - // The BPF compiler hides the fact that BPF instructions cannot directly - // access 64bit quantities. But policies are still advised to specify whether - // a system call expects a 32bit or a 64bit quantity. - enum ArgType { - // When passed as an argument to SandboxBPF::Cond(), TP_32BIT requests that - // the conditional test should operate on the 32bit part of the system call - // argument. - // On 64bit architectures, this verifies that user space did not pass - // a 64bit value as an argument to the system call. If it did, that will be - // interpreted as an attempt at breaking the sandbox and results in the - // program getting terminated. - // In other words, only perform a 32bit test, if you are sure this - // particular system call would never legitimately take a 64bit - // argument. - // Implementation detail: TP_32BIT does two things. 1) it restricts the - // conditional test to operating on the LSB only, and 2) it adds code to - // the BPF filter program verifying that the MSB the kernel received from - // user space is either 0, or 0xFFFFFFFF; the latter is acceptable, iff bit - // 31 was set in the system call argument. It deals with 32bit arguments - // having been sign extended. - TP_32BIT, - - // When passed as an argument to SandboxBPF::Cond(), TP_64BIT requests that - // the conditional test should operate on the full 64bit argument. It is - // generally harmless to perform a 64bit test on 32bit systems, as the - // kernel will always see the top 32 bits of all arguments as zero'd out. - // This approach has the desirable property that for tests of pointer - // values, we can always use TP_64BIT no matter the host architecture. - // But of course, that also means, it is possible to write conditional - // policies that turn into no-ops on 32bit systems; this is by design. - TP_64BIT, - }; - - // Deprecated. - enum Operation { - // Test whether the system call argument is equal to the operand. - OP_EQUAL, - - // 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. - // The "ANY_BITS" variant performs this test: "arg & mask != 0" - // This implies that a mask of zero always results in a failing test. - OP_HAS_ALL_BITS, - OP_HAS_ANY_BITS, - }; - - enum ErrorType { - ET_INVALID, - ET_SIMPLE, - ET_TRAP, - ET_COND, - }; - - // We allow the default constructor, as it makes the ErrorCode class - // much easier to use. But if we ever encounter an invalid ErrorCode - // when compiling a BPF filter, we deliberately generate an invalid - // program that will get flagged both by our Verifier class and by - // the Linux kernel. - ErrorCode(); - explicit ErrorCode(int err); - - // For all practical purposes, ErrorCodes are treated as if they were - // structs. The copy constructor and assignment operator are trivial and - // we do not need to explicitly specify them. - // Most notably, it is in fact perfectly OK to directly copy the passed_ and - // failed_ field. They only ever get set by our private constructor, and the - // callers handle life-cycle management for these objects. - - // Destructor - ~ErrorCode() {} - - bool Equals(const ErrorCode& err) const; - bool LessThan(const ErrorCode& err) const; - - uint32_t err() const { return err_; } - ErrorType error_type() const { return error_type_; } - - 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_; } - const ErrorCode* passed() const { return passed_; } - const ErrorCode* failed() const { return failed_; } - - struct LessThan { - bool operator()(const ErrorCode& a, const ErrorCode& b) const { - return a.LessThan(b); - } - }; - private: - friend class PolicyCompiler; - - // If we are wrapping a callback, we must assign a unique id. This id is - // how the kernel tells us which one of our different SECCOMP_RET_TRAP - // cases has been triggered. - ErrorCode(uint16_t trap_id, - TrapRegistry::TrapFnc fnc, - const void* aux, - bool safe); - - // Some system calls require inspection of arguments. This constructor - // allows us to specify additional constraints. - ErrorCode(int argno, - ArgType width, - uint64_t mask, - uint64_t value, - const ErrorCode* passed, - const ErrorCode* failed); - - ErrorType error_type_; - - union { - // Fields needed for SECCOMP_RET_TRAP callbacks - struct { - TrapRegistry::TrapFnc fnc_; // Callback function and arg, if trap was - void* aux_; // triggered by the kernel's BPF filter. - bool safe_; // Keep sandbox active while calling fnc_() - }; - - // 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. - const ErrorCode* passed_; // Value to be returned if comparison passed, - const ErrorCode* failed_; // or if it failed. - }; - }; - - // 32bit field used for all possible types of ErrorCode values. This is - // the value that uniquely identifies any ErrorCode and it (typically) can - // be emitted directly into a BPF filter program. - uint32_t err_; + DISALLOW_IMPLICIT_CONSTRUCTORS(ErrorCode); }; } // namespace bpf_dsl diff --git a/sandbox/linux/bpf_dsl/errorcode_unittest.cc b/sandbox/linux/bpf_dsl/errorcode_unittest.cc deleted file mode 100644 index 7277ed6eac52c0..00000000000000 --- a/sandbox/linux/bpf_dsl/errorcode_unittest.cc +++ /dev/null @@ -1,120 +0,0 @@ -// Copyright (c) 2012 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "sandbox/linux/bpf_dsl/errorcode.h" - -#include - -#include "base/macros.h" -#include "sandbox/linux/bpf_dsl/bpf_dsl.h" -#include "sandbox/linux/bpf_dsl/policy.h" -#include "sandbox/linux/bpf_dsl/policy_compiler.h" -#include "sandbox/linux/bpf_dsl/test_trap_registry.h" -#include "sandbox/linux/system_headers/linux_seccomp.h" -#include "testing/gtest/include/gtest/gtest.h" - -namespace sandbox { -namespace bpf_dsl { -namespace { - -class DummyPolicy : public Policy { - public: - DummyPolicy() {} - ~DummyPolicy() override {} - - ResultExpr EvaluateSyscall(int sysno) const override { return Allow(); } - - private: - DISALLOW_COPY_AND_ASSIGN(DummyPolicy); -}; - -class ErrorCodeTest : public ::testing::Test { - protected: - ErrorCodeTest() - : policy_(), trap_registry_(), compiler_(&policy_, &trap_registry_) {} - ~ErrorCodeTest() override {} - - DummyPolicy policy_; - TestTrapRegistry trap_registry_; - PolicyCompiler compiler_; - - DISALLOW_COPY_AND_ASSIGN(ErrorCodeTest); -}; - -TEST_F(ErrorCodeTest, ErrnoConstructor) { - ErrorCode e0; - EXPECT_EQ(SECCOMP_RET_INVALID, e0.err()); - - ErrorCode e1(ErrorCode::ERR_ALLOWED); - EXPECT_EQ(SECCOMP_RET_ALLOW, e1.err()); - - ErrorCode e2(EPERM); - EXPECT_EQ(SECCOMP_RET_ERRNO + EPERM, e2.err()); - - ErrorCode e3 = compiler_.Trap(NULL, NULL, true /* safe */); - EXPECT_EQ(SECCOMP_RET_TRAP, (e3.err() & SECCOMP_RET_ACTION)); - - uint16_t data = 0xdead; - ErrorCode e4(ErrorCode::ERR_TRACE + data); - EXPECT_EQ(SECCOMP_RET_TRACE + data, e4.err()); -} - -TEST_F(ErrorCodeTest, InvalidSeccompRetTrace) { - // Should die if the trace data does not fit in 16 bits. - ASSERT_DEATH(ErrorCode e(ErrorCode::ERR_TRACE + (1 << 16)), - "Invalid use of ErrorCode object"); -} - -TEST_F(ErrorCodeTest, Trap) { - ErrorCode e0 = compiler_.Trap(NULL, "a", true /* safe */); - ErrorCode e1 = compiler_.Trap(NULL, "b", true /* safe */); - EXPECT_EQ((e0.err() & SECCOMP_RET_DATA) + 1, e1.err() & SECCOMP_RET_DATA); - - ErrorCode e2 = compiler_.Trap(NULL, "a", true /* safe */); - EXPECT_EQ(e0.err() & SECCOMP_RET_DATA, e2.err() & SECCOMP_RET_DATA); -} - -TEST_F(ErrorCodeTest, Equals) { - ErrorCode e1(ErrorCode::ERR_ALLOWED); - ErrorCode e2(ErrorCode::ERR_ALLOWED); - EXPECT_TRUE(e1.Equals(e1)); - EXPECT_TRUE(e1.Equals(e2)); - EXPECT_TRUE(e2.Equals(e1)); - - ErrorCode e3(EPERM); - EXPECT_FALSE(e1.Equals(e3)); - - ErrorCode e4 = compiler_.Trap(NULL, "a", true /* safe */); - ErrorCode e5 = compiler_.Trap(NULL, "b", true /* safe */); - ErrorCode e6 = compiler_.Trap(NULL, "a", true /* safe */); - EXPECT_FALSE(e1.Equals(e4)); - EXPECT_FALSE(e3.Equals(e4)); - EXPECT_FALSE(e5.Equals(e4)); - EXPECT_TRUE(e6.Equals(e4)); -} - -TEST_F(ErrorCodeTest, LessThan) { - ErrorCode e1(ErrorCode::ERR_ALLOWED); - ErrorCode e2(ErrorCode::ERR_ALLOWED); - EXPECT_FALSE(e1.LessThan(e1)); - EXPECT_FALSE(e1.LessThan(e2)); - EXPECT_FALSE(e2.LessThan(e1)); - - ErrorCode e3(EPERM); - EXPECT_FALSE(e1.LessThan(e3)); - EXPECT_TRUE(e3.LessThan(e1)); - - ErrorCode e4 = compiler_.Trap(NULL, "a", true /* safe */); - ErrorCode e5 = compiler_.Trap(NULL, "b", true /* safe */); - ErrorCode e6 = compiler_.Trap(NULL, "a", true /* safe */); - EXPECT_TRUE(e1.LessThan(e4)); - EXPECT_TRUE(e3.LessThan(e4)); - EXPECT_TRUE(e4.LessThan(e5)); - EXPECT_FALSE(e4.LessThan(e6)); - EXPECT_FALSE(e6.LessThan(e4)); -} - -} // namespace -} // namespace bpf_dsl -} // namespace sandbox diff --git a/sandbox/linux/bpf_dsl/policy_compiler.cc b/sandbox/linux/bpf_dsl/policy_compiler.cc index ad9ccbab61170d..cb6f00c114587b 100644 --- a/sandbox/linux/bpf_dsl/policy_compiler.cc +++ b/sandbox/linux/bpf_dsl/policy_compiler.cc @@ -14,7 +14,6 @@ #include "sandbox/linux/bpf_dsl/bpf_dsl.h" #include "sandbox/linux/bpf_dsl/bpf_dsl_impl.h" #include "sandbox/linux/bpf_dsl/codegen.h" -#include "sandbox/linux/bpf_dsl/errorcode.h" #include "sandbox/linux/bpf_dsl/policy.h" #include "sandbox/linux/bpf_dsl/seccomp_macros.h" #include "sandbox/linux/bpf_dsl/syscall_set.h" @@ -91,7 +90,6 @@ PolicyCompiler::PolicyCompiler(const Policy* policy, TrapRegistry* registry) registry_(registry), escapepc_(0), panic_func_(DefaultPanic), - conds_(), gen_(), has_unsafe_traps_(HasUnsafeTraps(policy_)) { DCHECK(policy); @@ -185,7 +183,7 @@ CodeGen::Node PolicyCompiler::MaybeAddEscapeHatch(CodeGen::Node rest) { } CodeGen::Node PolicyCompiler::DispatchSyscall() { - // Evaluate all possible system calls and group their ErrorCodes into + // Evaluate all possible system calls and group their Nodes into // ranges of identical codes. Ranges ranges; FindRanges(&ranges); @@ -225,7 +223,7 @@ void PolicyCompiler::FindRanges(Ranges* ranges) { // int32_t, but BPF instructions always operate on unsigned quantities. We // deal with this disparity by enumerating from MIN_SYSCALL to MAX_SYSCALL, // and then verifying that the rest of the number range (both positive and - // negative) all return the same ErrorCode. + // negative) all return the same Node. const CodeGen::Node invalid_node = CompileResult(policy_->InvalidSyscall()); uint32_t old_sysnum = 0; CodeGen::Node old_node = @@ -277,68 +275,54 @@ CodeGen::Node PolicyCompiler::AssembleJumpTable(Ranges::const_iterator start, } CodeGen::Node PolicyCompiler::CompileResult(const ResultExpr& res) { - return RetExpression(res->Compile(this)); + return res->Compile(this); } -CodeGen::Node PolicyCompiler::RetExpression(const ErrorCode& err) { - switch (err.error_type()) { - case ErrorCode::ET_COND: - return CondExpression(err); - case ErrorCode::ET_SIMPLE: - case ErrorCode::ET_TRAP: - return gen_.MakeInstruction(BPF_RET + BPF_K, err.err()); - default: - LOG(FATAL) - << "ErrorCode is not suitable for returning from a BPF program"; - return CodeGen::kNullNode; - } -} - -CodeGen::Node PolicyCompiler::CondExpression(const ErrorCode& cond) { - // Sanity check that |cond| makes sense. - CHECK(cond.argno_ >= 0 && cond.argno_ < 6) << "Invalid argument number " - << cond.argno_; - CHECK(cond.width_ == ErrorCode::TP_32BIT || - cond.width_ == ErrorCode::TP_64BIT) - << "Invalid argument width " << cond.width_; - CHECK_NE(0U, cond.mask_) << "Zero mask is invalid"; - CHECK_EQ(cond.value_, cond.value_ & cond.mask_) - << "Value contains masked out bits"; +CodeGen::Node PolicyCompiler::MaskedEqual(int argno, + size_t width, + uint64_t mask, + uint64_t value, + CodeGen::Node passed, + CodeGen::Node failed) { + // Sanity check that arguments make sense. + CHECK(argno >= 0 && argno < 6) << "Invalid argument number " << argno; + CHECK(width == 4 || width == 8) << "Invalid argument width " << width; + CHECK_NE(0U, mask) << "Zero mask is invalid"; + CHECK_EQ(value, value & mask) << "Value contains masked out bits"; if (sizeof(void*) == 4) { - CHECK_EQ(ErrorCode::TP_32BIT, cond.width_) - << "Invalid width on 32-bit platform"; + CHECK_EQ(4U, width) << "Invalid width on 32-bit platform"; } - if (cond.width_ == ErrorCode::TP_32BIT) { - CHECK_EQ(0U, cond.mask_ >> 32) << "Mask exceeds argument size"; - CHECK_EQ(0U, cond.value_ >> 32) << "Value exceeds argument size"; + if (width == 4) { + CHECK_EQ(0U, mask >> 32) << "Mask exceeds argument size"; + CHECK_EQ(0U, value >> 32) << "Value exceeds argument size"; } - CodeGen::Node passed = RetExpression(*cond.passed_); - CodeGen::Node failed = RetExpression(*cond.failed_); - // We want to emit code to check "(arg & mask) == value" where arg, mask, and // value are 64-bit values, but the BPF machine is only 32-bit. We implement // this by independently testing the upper and lower 32-bits and continuing to // |passed| if both evaluate true, or to |failed| if either evaluate false. - return CondExpressionHalf(cond, - UpperHalf, - CondExpressionHalf(cond, LowerHalf, passed, failed), - failed); + return MaskedEqualHalf(argno, width, mask, value, ArgHalf::UPPER, + MaskedEqualHalf(argno, width, mask, value, + ArgHalf::LOWER, passed, failed), + failed); } -CodeGen::Node PolicyCompiler::CondExpressionHalf(const ErrorCode& cond, - ArgHalf half, - CodeGen::Node passed, - CodeGen::Node failed) { - if (cond.width_ == ErrorCode::TP_32BIT && half == UpperHalf) { +CodeGen::Node PolicyCompiler::MaskedEqualHalf(int argno, + size_t width, + uint64_t full_mask, + uint64_t full_value, + ArgHalf half, + CodeGen::Node passed, + CodeGen::Node failed) { + if (width == 4 && half == ArgHalf::UPPER) { // Special logic for sanity checking the upper 32-bits of 32-bit system // call arguments. // TODO(mdempsky): Compile Unexpected64bitArgument() just per program. - CodeGen::Node invalid_64bit = RetExpression(Unexpected64bitArgument()); + CodeGen::Node invalid_64bit = Unexpected64bitArgument(); - const uint32_t upper = SECCOMP_ARG_MSB_IDX(cond.argno_); - const uint32_t lower = SECCOMP_ARG_LSB_IDX(cond.argno_); + const uint32_t upper = SECCOMP_ARG_MSB_IDX(argno); + const uint32_t lower = SECCOMP_ARG_LSB_IDX(argno); if (sizeof(void*) == 4) { // On 32-bit platforms, the upper 32-bits should always be 0: @@ -381,10 +365,11 @@ CodeGen::Node PolicyCompiler::CondExpressionHalf(const ErrorCode& cond, invalid_64bit))); } - const uint32_t idx = (half == UpperHalf) ? SECCOMP_ARG_MSB_IDX(cond.argno_) - : SECCOMP_ARG_LSB_IDX(cond.argno_); - const uint32_t mask = (half == UpperHalf) ? cond.mask_ >> 32 : cond.mask_; - const uint32_t value = (half == UpperHalf) ? cond.value_ >> 32 : cond.value_; + const uint32_t idx = (half == ArgHalf::UPPER) ? SECCOMP_ARG_MSB_IDX(argno) + : SECCOMP_ARG_LSB_IDX(argno); + const uint32_t mask = (half == ArgHalf::UPPER) ? full_mask >> 32 : full_mask; + const uint32_t value = + (half == ArgHalf::UPPER) ? full_value >> 32 : full_value; // Emit a suitable instruction sequence for (arg & mask) == value. @@ -439,12 +424,12 @@ CodeGen::Node PolicyCompiler::CondExpressionHalf(const ErrorCode& cond, BPF_JMP + BPF_JEQ + BPF_K, value, passed, failed))); } -ErrorCode PolicyCompiler::Unexpected64bitArgument() { - return panic_func_("Unexpected 64bit argument detected")->Compile(this); +CodeGen::Node PolicyCompiler::Unexpected64bitArgument() { + return CompileResult(panic_func_("Unexpected 64bit argument detected")); } -ErrorCode PolicyCompiler::Error(int err) { - if (has_unsafe_traps_) { +CodeGen::Node PolicyCompiler::Return(uint32_t ret) { + if (has_unsafe_traps_ && (ret & SECCOMP_RET_ACTION) == SECCOMP_RET_ERRNO) { // When inside an UnsafeTrap() callback, we want to allow all system calls. // This means, we must conditionally disable the sandbox -- and that's not // something that kernel-side BPF filters can do, as they cannot inspect @@ -454,17 +439,18 @@ ErrorCode PolicyCompiler::Error(int err) { // The performance penalty for this extra round-trip to user-space is not // actually that bad, as we only ever pay it for denied system calls; and a // typical program has very few of these. - return Trap(ReturnErrno, reinterpret_cast(err), true); + return Trap(ReturnErrno, reinterpret_cast(ret & SECCOMP_RET_DATA), + true); } - return ErrorCode(err); + return gen_.MakeInstruction(BPF_RET + BPF_K, ret); } -ErrorCode PolicyCompiler::Trap(TrapRegistry::TrapFnc fnc, - const void* aux, - bool safe) { +CodeGen::Node PolicyCompiler::Trap(TrapRegistry::TrapFnc fnc, + const void* aux, + bool safe) { uint16_t trap_id = registry_->Add(fnc, aux, safe); - return ErrorCode(trap_id, fnc, aux, safe); + return gen_.MakeInstruction(BPF_RET + BPF_K, SECCOMP_RET_TRAP + trap_id); } bool PolicyCompiler::IsRequiredForUnsafeTrap(int sysno) { @@ -476,19 +462,5 @@ bool PolicyCompiler::IsRequiredForUnsafeTrap(int sysno) { return false; } -ErrorCode PolicyCompiler::CondMaskedEqual(int argno, - ErrorCode::ArgType width, - uint64_t mask, - uint64_t value, - const ErrorCode& passed, - const ErrorCode& failed) { - return ErrorCode(argno, - width, - mask, - value, - &*conds_.insert(passed).first, - &*conds_.insert(failed).first); -} - } // namespace bpf_dsl } // namespace sandbox diff --git a/sandbox/linux/bpf_dsl/policy_compiler.h b/sandbox/linux/bpf_dsl/policy_compiler.h index f0979417663246..e81404fb57073d 100644 --- a/sandbox/linux/bpf_dsl/policy_compiler.h +++ b/sandbox/linux/bpf_dsl/policy_compiler.h @@ -7,15 +7,12 @@ #include -#include -#include #include #include "base/macros.h" #include "base/memory/scoped_ptr.h" #include "sandbox/linux/bpf_dsl/bpf_dsl_forward.h" #include "sandbox/linux/bpf_dsl/codegen.h" -#include "sandbox/linux/bpf_dsl/errorcode.h" #include "sandbox/linux/bpf_dsl/trap_registry.h" #include "sandbox/sandbox_export.h" @@ -47,49 +44,42 @@ class SANDBOX_EXPORT PolicyCompiler { // TODO(mdempsky): Move this into Policy? void SetPanicFunc(PanicFunc panic_func); - // Error returns an ErrorCode to indicate the system call should fail with - // the specified error number. - ErrorCode Error(int err); - - // Trap returns an ErrorCode to indicate the system call should - // instead invoke a trap handler. - ErrorCode Trap(TrapRegistry::TrapFnc fnc, const void* aux, bool safe); - // UnsafeTraps require some syscalls to always be allowed. // This helper function returns true for these calls. static bool IsRequiredForUnsafeTrap(int sysno); - // 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. + // Functions below are meant for use within bpf_dsl itself. + + // Return returns a CodeGen::Node that returns the specified seccomp + // return value. + CodeGen::Node Return(uint32_t ret); + + // Trap returns a CodeGen::Node to indicate the system call should + // instead invoke a trap handler. + CodeGen::Node Trap(TrapRegistry::TrapFnc fnc, const void* aux, bool safe); + + // MaskedEqual returns a CodeGen::Node that represents a conditional branch. // 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) + // to "value"; if equal, then "passed" will be executed, otherwise "failed". + // If "width" is 4, 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, + // the same as any other ABI violation (i.e., it panics). + CodeGen::Node MaskedEqual(int argno, + size_t width, uint64_t mask, uint64_t value, - const ErrorCode& passed, - const ErrorCode& failed); - - // Returns the fatal ErrorCode that is used to indicate that somebody - // attempted to pass a 64bit value in a 32bit system call argument. - // This method is primarily needed for testing purposes. - ErrorCode Unexpected64bitArgument(); + CodeGen::Node passed, + CodeGen::Node failed); private: struct Range; typedef std::vector Ranges; - typedef std::set Conds; - // Used by CondExpressionHalf to track which half of the argument it's + // Used by MaskedEqualHalf to track which half of the argument it's // emitting instructions for. - enum ArgHalf { - LowerHalf, - UpperHalf, + enum class ArgHalf { + LOWER, + UPPER, }; // Compile the configured policy into a complete instruction sequence. @@ -118,7 +108,8 @@ class SANDBOX_EXPORT PolicyCompiler { // Finds all the ranges of system calls that need to be handled. Ranges are // sorted in ascending order of system call numbers. There are no gaps in the - // ranges. System calls with identical ErrorCodes are coalesced into a single + // ranges. System calls with identical CodeGen::Nodes are coalesced into a + // single // range. void FindRanges(Ranges* ranges); @@ -131,32 +122,25 @@ class SANDBOX_EXPORT PolicyCompiler { // CodeGen node. CodeGen::Node CompileResult(const ResultExpr& res); - // Returns a BPF program snippet that makes the BPF filter program exit - // with the given ErrorCode "err". N.B. the ErrorCode may very well be a - // conditional expression; if so, this function will recursively call - // CondExpression() and possibly RetExpression() to build a complex set of - // instructions. - CodeGen::Node RetExpression(const ErrorCode& err); - - // Returns a BPF program that evaluates the conditional expression in - // "cond" and returns the appropriate value from the BPF filter program. - // This function recursively calls RetExpression(); it should only ever be - // called from RetExpression(). - CodeGen::Node CondExpression(const ErrorCode& cond); - // Returns a BPF program that evaluates half of a conditional expression; // it should only ever be called from CondExpression(). - CodeGen::Node CondExpressionHalf(const ErrorCode& cond, - ArgHalf half, - CodeGen::Node passed, - CodeGen::Node failed); + CodeGen::Node MaskedEqualHalf(int argno, + size_t width, + uint64_t full_mask, + uint64_t full_value, + ArgHalf half, + CodeGen::Node passed, + CodeGen::Node failed); + + // Returns the fatal CodeGen::Node that is used to indicate that somebody + // attempted to pass a 64bit value in a 32bit system call argument. + CodeGen::Node Unexpected64bitArgument(); const Policy* policy_; TrapRegistry* registry_; uint64_t escapepc_; PanicFunc panic_func_; - Conds conds_; CodeGen gen_; bool has_unsafe_traps_; diff --git a/sandbox/linux/sandbox_linux.gypi b/sandbox/linux/sandbox_linux.gypi index 289d6fbad5f36e..8389512cfd2fa8 100644 --- a/sandbox/linux/sandbox_linux.gypi +++ b/sandbox/linux/sandbox_linux.gypi @@ -135,7 +135,6 @@ 'bpf_dsl/codegen.cc', 'bpf_dsl/codegen.h', 'bpf_dsl/cons.h', - 'bpf_dsl/errorcode.cc', 'bpf_dsl/errorcode.h', 'bpf_dsl/linux_syscall_ranges.h', 'bpf_dsl/policy.cc', diff --git a/sandbox/linux/sandbox_linux_nacl_nonsfi.gyp b/sandbox/linux/sandbox_linux_nacl_nonsfi.gyp index f131c0e9d92f9a..0b40d1cb9c77e2 100644 --- a/sandbox/linux/sandbox_linux_nacl_nonsfi.gyp +++ b/sandbox/linux/sandbox_linux_nacl_nonsfi.gyp @@ -29,7 +29,6 @@ # nacl_helper_nonsfi's sandbox implementation. 'bpf_dsl/bpf_dsl.cc', 'bpf_dsl/codegen.cc', - 'bpf_dsl/errorcode.cc', 'bpf_dsl/policy.cc', 'bpf_dsl/policy_compiler.cc', 'bpf_dsl/syscall_set.cc', diff --git a/sandbox/linux/sandbox_linux_test_sources.gypi b/sandbox/linux/sandbox_linux_test_sources.gypi index d25aabfdd80df9..612814e1d48e14 100644 --- a/sandbox/linux/sandbox_linux_test_sources.gypi +++ b/sandbox/linux/sandbox_linux_test_sources.gypi @@ -45,7 +45,6 @@ 'bpf_dsl/cons_unittest.cc', 'bpf_dsl/dump_bpf.cc', 'bpf_dsl/dump_bpf.h', - 'bpf_dsl/errorcode_unittest.cc', 'bpf_dsl/syscall_set_unittest.cc', 'bpf_dsl/test_trap_registry.cc', 'bpf_dsl/test_trap_registry.h',