Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(287)

Issues Search
    My Issues | Starred     Open | Closed | All

Unified Diff: sandbox/linux/seccomp-bpf/verifier.cc

Issue 12223109: SECCOMP-BPF: Refactor the BPF sandbox API to use fewer "static" fields and methods. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Make sure unnamed namespaces are always top-level Created 7 years, 10 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« sandbox/linux/seccomp-bpf/util.cc ('K') | « sandbox/linux/seccomp-bpf/verifier.h ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: sandbox/linux/seccomp-bpf/verifier.cc
diff --git a/sandbox/linux/seccomp-bpf/verifier.cc b/sandbox/linux/seccomp-bpf/verifier.cc
index 06143b9c20e5ac45723f23e9718295b911627c0c..60c0eab20d613ef0fe165098dc66e2945d9e93fc 100644
--- a/sandbox/linux/seccomp-bpf/verifier.cc
+++ b/sandbox/linux/seccomp-bpf/verifier.cc
@@ -2,86 +2,76 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
+#include <string.h>
+
#include "sandbox/linux/seccomp-bpf/sandbox_bpf.h"
#include "sandbox/linux/seccomp-bpf/syscall_iterator.h"
#include "sandbox/linux/seccomp-bpf/verifier.h"
-namespace playground2 {
+namespace {
-bool Verifier::VerifyBPF(const std::vector<struct sock_filter>& program,
- const Sandbox::Evaluators& evaluators,
- const char **err) {
- *err = NULL;
- if (evaluators.size() != 1) {
- *err = "Not implemented";
- return false;
- }
- Sandbox::EvaluateSyscall evaluate_syscall = evaluators.begin()->first;
- void *aux = evaluators.begin()->second;
- for (SyscallIterator iter(false); !iter.Done(); ) {
- uint32_t sysnum = iter.Next();
- // We ideally want to iterate over the full system call range and values
- // just above and just below this range. This gives us the full result set
- // of the "evaluators".
- // On Intel systems, this can fail in a surprising way, as a cleared bit 30
- // indicates either i386 or x86-64; and a set bit 30 indicates x32. And
- // unless we pay attention to setting this bit correctly, an early check in
- // our BPF program will make us fail with a misleading error code.
- struct arch_seccomp_data data = { static_cast<int>(sysnum),
- static_cast<uint32_t>(SECCOMP_ARCH) };
-#if defined(__i386__) || defined(__x86_64__)
-#if defined(__x86_64__) && defined(__ILP32__)
- if (!(sysnum & 0x40000000u)) {
- continue;
- }
-#else
- if (sysnum & 0x40000000u) {
- continue;
- }
-#endif
-#endif
- ErrorCode code = evaluate_syscall(sysnum, aux);
- if (!VerifyErrorCode(program, &data, code, code, err)) {
- return false;
- }
+using playground2::ErrorCode;
+using playground2::Sandbox;
+using playground2::Verifier;
+using playground2::arch_seccomp_data;
+
+struct State {
+ State(const std::vector<struct sock_filter>& p,
+ const struct arch_seccomp_data& d) :
+ program(p),
+ data(d),
+ ip(0),
+ accumulator(0),
+ acc_is_valid(false) {
}
- return true;
-}
+ const std::vector<struct sock_filter>& program;
+ const struct arch_seccomp_data& data;
+ unsigned int ip;
+ uint32_t accumulator;
+ bool acc_is_valid;
+
+ private:
+ DISALLOW_IMPLICIT_CONSTRUCTORS(State);
+};
-uint32_t Verifier::EvaluateErrorCode(const ErrorCode& code,
- const struct arch_seccomp_data& data) {
- if (code.error_type_ == ErrorCode::ET_SIMPLE ||
- code.error_type_ == ErrorCode::ET_TRAP) {
- return code.err_;
- } else if (code.error_type_ == ErrorCode::ET_COND) {
- if (code.width_ == ErrorCode::TP_32BIT &&
- (data.args[code.argno_] >> 32) &&
- (data.args[code.argno_]&0xFFFFFFFF80000000ull)!=0xFFFFFFFF80000000ull){
- return Sandbox::Unexpected64bitArgument().err();
+uint32_t EvaluateErrorCode(Sandbox *sandbox, const ErrorCode& code,
+ const struct arch_seccomp_data& data) {
+ if (code.error_type() == ErrorCode::ET_SIMPLE ||
+ code.error_type() == ErrorCode::ET_TRAP) {
+ return code.err();
+ } else if (code.error_type() == ErrorCode::ET_COND) {
+ if (code.width() == ErrorCode::TP_32BIT &&
+ (data.args[code.argno()] >> 32) &&
+ (data.args[code.argno()] & 0xFFFFFFFF80000000ull) !=
+ 0xFFFFFFFF80000000ull) {
+ return sandbox->Unexpected64bitArgument().err();
}
- switch (code.op_) {
+ switch (code.op()) {
case ErrorCode::OP_EQUAL:
- return EvaluateErrorCode((code.width_ == ErrorCode::TP_32BIT
- ? uint32_t(data.args[code.argno_])
- : data.args[code.argno_]) == code.value_
- ? *code.passed_
- : *code.failed_,
+ return EvaluateErrorCode(sandbox,
+ (code.width() == ErrorCode::TP_32BIT
+ ? uint32_t(data.args[code.argno()])
+ : data.args[code.argno()]) == code.value()
+ ? *code.passed()
+ : *code.failed(),
data);
case ErrorCode::OP_HAS_ALL_BITS:
- return EvaluateErrorCode(((code.width_ == ErrorCode::TP_32BIT
- ? uint32_t(data.args[code.argno_])
- : data.args[code.argno_]) & code.value_)
- == code.value_
- ? *code.passed_
- : *code.failed_,
+ return EvaluateErrorCode(sandbox,
+ ((code.width() == ErrorCode::TP_32BIT
+ ? uint32_t(data.args[code.argno()])
+ : data.args[code.argno()]) & code.value())
+ == code.value()
+ ? *code.passed()
+ : *code.failed(),
data);
case ErrorCode::OP_HAS_ANY_BITS:
- return EvaluateErrorCode((code.width_ == ErrorCode::TP_32BIT
- ? uint32_t(data.args[code.argno_])
- : data.args[code.argno_]) & code.value_
- ? *code.passed_
- : *code.failed_,
+ return EvaluateErrorCode(sandbox,
+ (code.width() == ErrorCode::TP_32BIT
+ ? uint32_t(data.args[code.argno()])
+ : data.args[code.argno()]) & code.value()
+ ? *code.passed()
+ : *code.failed(),
data);
default:
return SECCOMP_RET_INVALID;
@@ -91,19 +81,20 @@ uint32_t Verifier::EvaluateErrorCode(const ErrorCode& code,
}
}
-bool Verifier::VerifyErrorCode(const std::vector<struct sock_filter>& program,
- struct arch_seccomp_data *data,
- const ErrorCode& root_code,
- const ErrorCode& code,
- const char **err) {
- if (code.error_type_ == ErrorCode::ET_SIMPLE ||
- code.error_type_ == ErrorCode::ET_TRAP) {
- uint32_t computed_ret = EvaluateBPF(program, *data, err);
+bool VerifyErrorCode(Sandbox *sandbox,
+ const std::vector<struct sock_filter>& program,
+ struct arch_seccomp_data *data,
+ const ErrorCode& root_code,
+ const ErrorCode& code,
+ const char **err) {
+ if (code.error_type() == ErrorCode::ET_SIMPLE ||
+ code.error_type() == ErrorCode::ET_TRAP) {
+ uint32_t computed_ret = Verifier::EvaluateBPF(program, *data, err);
if (*err) {
return false;
- } else if (computed_ret != EvaluateErrorCode(root_code, *data)) {
+ } else if (computed_ret != EvaluateErrorCode(sandbox, root_code, *data)) {
// For efficiency's sake, we'd much rather compare "computed_ret"
- // against "code.err_". This works most of the time, but it doesn't
+ // against "code.err()". This works most of the time, but it doesn't
// always work for nested conditional expressions. The test values
// that we generate on the fly to probe expressions can trigger
// code flow decisions in multiple nodes of the decision tree, and the
@@ -112,32 +103,34 @@ bool Verifier::VerifyErrorCode(const std::vector<struct sock_filter>& program,
*err = "Exit code from BPF program doesn't match";
return false;
}
- } else if (code.error_type_ == ErrorCode::ET_COND) {
- if (code.argno_ < 0 || code.argno_ >= 6) {
+ } else if (code.error_type() == ErrorCode::ET_COND) {
+ if (code.argno() < 0 || code.argno() >= 6) {
*err = "Invalid argument number in error code";
return false;
}
- switch (code.op_) {
+ switch (code.op()) {
case ErrorCode::OP_EQUAL:
// Verify that we can check a 32bit value (or the LSB of a 64bit value)
// for equality.
- data->args[code.argno_] = code.value_;
- if (!VerifyErrorCode(program, data, root_code, *code.passed_, err)) {
+ data->args[code.argno()] = code.value();
+ if (!VerifyErrorCode(sandbox, program, data, root_code,
+ *code.passed(), err)) {
return false;
}
// Change the value to no longer match and verify that this is detected
// as an inequality.
- data->args[code.argno_] = code.value_ ^ 0x55AA55AA;
- if (!VerifyErrorCode(program, data, root_code, *code.failed_, err)) {
+ data->args[code.argno()] = code.value() ^ 0x55AA55AA;
+ if (!VerifyErrorCode(sandbox, program, data, root_code,
+ *code.failed(), err)) {
return false;
}
// BPF programs can only ever operate on 32bit values. So, we have
// generated additional BPF instructions that inspect the MSB. Verify
// that they behave as intended.
- if (code.width_ == ErrorCode::TP_32BIT) {
- if (code.value_ >> 32) {
+ if (code.width() == ErrorCode::TP_32BIT) {
+ if (code.value() >> 32) {
SANDBOX_DIE("Invalid comparison of a 32bit system call argument "
"against a 64bit constant; this test is always false.");
}
@@ -145,9 +138,10 @@ bool Verifier::VerifyErrorCode(const std::vector<struct sock_filter>& program,
// If the system call argument was intended to be a 32bit parameter,
// verify that it is a fatal error if a 64bit value is ever passed
// here.
- data->args[code.argno_] = 0x100000000ull;
- if (!VerifyErrorCode(program, data, root_code,
- Sandbox::Unexpected64bitArgument(), err)) {
+ data->args[code.argno()] = 0x100000000ull;
+ if (!VerifyErrorCode(sandbox, program, data, root_code,
+ sandbox->Unexpected64bitArgument(),
+ err)) {
return false;
}
} else {
@@ -157,8 +151,9 @@ bool Verifier::VerifyErrorCode(const std::vector<struct sock_filter>& program,
// We only need to verify the behavior of the inequality test. We
// know that the equality test already passed, as unlike the kernel
// the Verifier does operate on 64bit quantities.
- data->args[code.argno_] = code.value_ ^ 0x55AA55AA00000000ull;
- if (!VerifyErrorCode(program, data, root_code, *code.failed_, err)) {
+ data->args[code.argno()] = code.value() ^ 0x55AA55AA00000000ull;
+ if (!VerifyErrorCode(sandbox, program, data, root_code,
+ *code.failed(), err)) {
return false;
}
}
@@ -172,37 +167,37 @@ bool Verifier::VerifyErrorCode(const std::vector<struct sock_filter>& program,
// ones.
{
// Testing "any" bits against a zero mask is always false. So, there
- // are some cases, where we expect tests to take the "failed_" branch
- // even though this is a test that normally should take "passed_".
+ // are some cases, where we expect tests to take the "failed()" branch
+ // even though this is a test that normally should take "passed()".
const ErrorCode& passed =
- (!code.value_ && code.op_ == ErrorCode::OP_HAS_ANY_BITS) ||
+ (!code.value() && code.op() == ErrorCode::OP_HAS_ANY_BITS) ||
// On a 32bit system, it is impossible to pass a 64bit value as a
// system call argument. So, some additional tests always evaluate
// as false.
- ((code.value_ & ~uint64_t(uintptr_t(-1))) &&
- code.op_ == ErrorCode::OP_HAS_ALL_BITS) ||
- (code.value_ && !(code.value_ & uintptr_t(-1)) &&
- code.op_ == ErrorCode::OP_HAS_ANY_BITS)
+ ((code.value() & ~uint64_t(uintptr_t(-1))) &&
+ code.op() == ErrorCode::OP_HAS_ALL_BITS) ||
+ (code.value() && !(code.value() & uintptr_t(-1)) &&
+ code.op() == ErrorCode::OP_HAS_ANY_BITS)
- ? *code.failed_ : *code.passed_;
+ ? *code.failed() : *code.passed();
// Similary, testing for "all" bits in a zero mask is always true. So,
// some cases pass despite them normally failing.
const ErrorCode& failed =
- !code.value_ && code.op_ == ErrorCode::OP_HAS_ALL_BITS
- ? *code.passed_ : *code.failed_;
+ !code.value() && code.op() == ErrorCode::OP_HAS_ALL_BITS
+ ? *code.passed() : *code.failed();
- data->args[code.argno_] = code.value_ & uintptr_t(-1);
- if (!VerifyErrorCode(program, data, root_code, passed, err)) {
+ data->args[code.argno()] = code.value() & uintptr_t(-1);
+ if (!VerifyErrorCode(sandbox, program, data, root_code, passed, err)) {
return false;
}
- data->args[code.argno_] = uintptr_t(-1);
- if (!VerifyErrorCode(program, data, root_code, passed, err)) {
+ data->args[code.argno()] = uintptr_t(-1);
+ if (!VerifyErrorCode(sandbox, program, data, root_code, passed, err)) {
return false;
}
- data->args[code.argno_] = 0;
- if (!VerifyErrorCode(program, data, root_code, failed, err)) {
+ data->args[code.argno()] = 0;
+ if (!VerifyErrorCode(sandbox, program, data, root_code, failed, err)) {
return false;
}
}
@@ -218,55 +213,7 @@ bool Verifier::VerifyErrorCode(const std::vector<struct sock_filter>& program,
return true;
}
-uint32_t Verifier::EvaluateBPF(const std::vector<struct sock_filter>& program,
- const struct arch_seccomp_data& data,
- const char **err) {
- *err = NULL;
- if (program.size() < 1 || program.size() >= SECCOMP_MAX_PROGRAM_SIZE) {
- *err = "Invalid program length";
- return 0;
- }
- for (State state(program, data); !*err; ++state.ip) {
- if (state.ip >= program.size()) {
- *err = "Invalid instruction pointer in BPF program";
- break;
- }
- const struct sock_filter& insn = program[state.ip];
- switch (BPF_CLASS(insn.code)) {
- case BPF_LD:
- Ld(&state, insn, err);
- break;
- case BPF_JMP:
- Jmp(&state, insn, err);
- break;
- case BPF_RET: {
- uint32_t r = Ret(&state, insn, err);
- switch (r & SECCOMP_RET_ACTION) {
- case SECCOMP_RET_TRAP:
- case SECCOMP_RET_ERRNO:
- case SECCOMP_RET_ALLOW:
- break;
- case SECCOMP_RET_KILL: // We don't ever generate this
- case SECCOMP_RET_TRACE: // We don't ever generate this
- case SECCOMP_RET_INVALID: // Should never show up in BPF program
- default:
- *err = "Unexpected return code found in BPF program";
- return 0;
- }
- return r; }
- case BPF_ALU:
- Alu(&state, insn, err);
- break;
- default:
- *err = "Unexpected instruction in BPF program";
- break;
- }
- }
- return 0;
-}
-
-void Verifier::Ld(State *state, const struct sock_filter& insn,
- const char **err) {
+void Ld(State *state, const struct sock_filter& insn, const char **err) {
if (BPF_SIZE(insn.code) != BPF_W ||
BPF_MODE(insn.code) != BPF_ABS) {
*err = "Invalid BPF_LD instruction";
@@ -285,8 +232,7 @@ void Verifier::Ld(State *state, const struct sock_filter& insn,
return;
}
-void Verifier::Jmp(State *state, const struct sock_filter& insn,
- const char **err) {
+void Jmp(State *state, const struct sock_filter& insn, const char **err) {
if (BPF_OP(insn.code) == BPF_JA) {
if (state->ip + insn.k + 1 >= state->program.size() ||
state->ip + insn.k + 1 <= state->ip) {
@@ -337,8 +283,7 @@ void Verifier::Jmp(State *state, const struct sock_filter& insn,
}
}
-uint32_t Verifier::Ret(State *, const struct sock_filter& insn,
- const char **err) {
+uint32_t Ret(State *, const struct sock_filter& insn, const char **err) {
if (BPF_SRC(insn.code) != BPF_K) {
*err = "Invalid BPF_RET instruction";
return 0;
@@ -346,8 +291,7 @@ uint32_t Verifier::Ret(State *, const struct sock_filter& insn,
return insn.k;
}
-void Verifier::Alu(State *state, const struct sock_filter& insn,
- const char **err) {
+void Alu(State *state, const struct sock_filter& insn, const char **err) {
if (BPF_OP(insn.code) == BPF_NEG) {
state->accumulator = -state->accumulator;
return;
@@ -410,5 +354,96 @@ void Verifier::Alu(State *state, const struct sock_filter& insn,
}
}
+} // namespace
+
+namespace playground2 {
+
+bool Verifier::VerifyBPF(Sandbox *sandbox,
+ const std::vector<struct sock_filter>& program,
+ const Sandbox::Evaluators& evaluators,
+ const char **err) {
+ *err = NULL;
+ if (evaluators.size() != 1) {
+ *err = "Not implemented";
+ return false;
+ }
+ Sandbox::EvaluateSyscall evaluate_syscall = evaluators.begin()->first;
+ void *aux = evaluators.begin()->second;
+ for (SyscallIterator iter(false); !iter.Done(); ) {
+ uint32_t sysnum = iter.Next();
+ // We ideally want to iterate over the full system call range and values
+ // just above and just below this range. This gives us the full result set
+ // of the "evaluators".
+ // On Intel systems, this can fail in a surprising way, as a cleared bit 30
+ // indicates either i386 or x86-64; and a set bit 30 indicates x32. And
+ // unless we pay attention to setting this bit correctly, an early check in
+ // our BPF program will make us fail with a misleading error code.
+ struct arch_seccomp_data data = { static_cast<int>(sysnum),
+ static_cast<uint32_t>(SECCOMP_ARCH) };
+#if defined(__i386__) || defined(__x86_64__)
+#if defined(__x86_64__) && defined(__ILP32__)
+ if (!(sysnum & 0x40000000u)) {
+ continue;
+ }
+#else
+ if (sysnum & 0x40000000u) {
+ continue;
+ }
+#endif
+#endif
+ ErrorCode code = evaluate_syscall(sandbox, sysnum, aux);
+ if (!VerifyErrorCode(sandbox, program, &data, code, code, err)) {
+ return false;
+ }
+ }
+ return true;
+}
+
+uint32_t Verifier::EvaluateBPF(const std::vector<struct sock_filter>& program,
+ const struct arch_seccomp_data& data,
+ const char **err) {
+ *err = NULL;
+ if (program.size() < 1 || program.size() >= SECCOMP_MAX_PROGRAM_SIZE) {
+ *err = "Invalid program length";
+ return 0;
+ }
+ for (State state(program, data); !*err; ++state.ip) {
+ if (state.ip >= program.size()) {
+ *err = "Invalid instruction pointer in BPF program";
+ break;
+ }
+ const struct sock_filter& insn = program[state.ip];
+ switch (BPF_CLASS(insn.code)) {
+ case BPF_LD:
+ Ld(&state, insn, err);
+ break;
+ case BPF_JMP:
+ Jmp(&state, insn, err);
+ break;
+ case BPF_RET: {
+ uint32_t r = Ret(&state, insn, err);
+ switch (r & SECCOMP_RET_ACTION) {
+ case SECCOMP_RET_TRAP:
+ case SECCOMP_RET_ERRNO:
+ case SECCOMP_RET_ALLOW:
+ break;
+ case SECCOMP_RET_KILL: // We don't ever generate this
+ case SECCOMP_RET_TRACE: // We don't ever generate this
+ case SECCOMP_RET_INVALID: // Should never show up in BPF program
+ default:
+ *err = "Unexpected return code found in BPF program";
+ return 0;
+ }
+ return r; }
+ case BPF_ALU:
+ Alu(&state, insn, err);
+ break;
+ default:
+ *err = "Unexpected instruction in BPF program";
+ break;
+ }
+ }
+ return 0;
+}
} // namespace
« sandbox/linux/seccomp-bpf/util.cc ('K') | « sandbox/linux/seccomp-bpf/verifier.h ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698