Skip to content

GitLab

T
tangled
Commit a5f20640 authored by mdempsky's avatar mdempsky Committed by Commit bot
Browse files
Options

Decouple CodeGen from ErrorCode 

CodeGen's only dependency on ErrorCode was one MakeInstruction()
overload, which can be removed by making the caller responsible for
converting the ErrorCode to a BPF_RET'able value. Fortunately, there
are only two uses of this function:

1. SandboxBPF::RetExpression() which is easily fixed by simply calling
ErrorCode::err() to get the ErrorCode's error value.

2. codegen_unittest.cc which is merely testing the BPF code generator,
and so any return values will do: no need to be seccomp-bpf return
values... so just replace them with simple integer values.  (While
here, change a few bare "BPF_RET" arguments to "BPF_RET + BPF_K" just
to be consistent.)

After this CL, CodeGen focuses solely on assembling programs for the
abstract BPF machine, while higher-level abstractions (primarily
SandboxBPF) are responsible for using it in a way that's semantically
meaningful for seccomp-bpf.

BUG=414363

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

Cr-Commit-Position: refs/heads/master@{#295192}
parent d117e910
Show whitespace changes
Inline Side-by-side
Showing with 36 additions and 46 deletions
sandbox/linux/seccomp-bpf/codegen.cc
View file @ a5f20640
......@@ -172,17 +172,6 @@ Instruction* CodeGen::MakeInstruction(uint16_t code,
}
}
Instruction* CodeGen::MakeInstruction(uint16_t code, const ErrorCode& err) {
if (BPF_CLASS(code) != BPF_RET) {
SANDBOX_DIE("ErrorCodes can only be used in return expressions");
}
if (err.error_type_ != ErrorCode::ET_SIMPLE &&
err.error_type_ != ErrorCode::ET_TRAP) {
SANDBOX_DIE("ErrorCode is not suitable for returning from a BPF program");
}
return MakeInstruction(code, err.err_);
}
Instruction* CodeGen::MakeInstruction(uint16_t code,
uint32_t k,
Instruction* jt,
......
sandbox/linux/seccomp-bpf/codegen.h
View file @ a5f20640
......@@ -13,7 +13,6 @@
namespace sandbox {
struct BasicBlock;
class ErrorCode;
struct Instruction;
typedef std::vector<Instruction*> Instructions;
......@@ -43,7 +42,8 @@ typedef std::map<const BasicBlock*, int> IncomingBranches;
// gen.MakeInstruction(BPF_JMP+BPF_EQ+BPF_K, __NR_getpid,
// Trap(GetPidHandler, NULL), NULL);
// gen.JoinInstructions(branch,
// gen.MakeInstruction(BPF_RET+BPF_K, ErrorCode(ErrorCode::ERR_ALLOWED)));
// gen.MakeInstruction(BPF_RET+BPF_K,
// ErrorCode(ErrorCode::ERR_ALLOWED).err()));
//
// // Simplified code follows; in practice, it is important to avoid calling
// // any C++ destructors after starting the sandbox.
......@@ -69,7 +69,6 @@ class SANDBOX_EXPORT CodeGen {
Instruction* MakeInstruction(uint16_t code,
uint32_t k,
Instruction* next = NULL);
Instruction* MakeInstruction(uint16_t code, const ErrorCode& err);
Instruction* MakeInstruction(uint16_t code,
uint32_t k,
Instruction* jt,
......
sandbox/linux/seccomp-bpf/codegen_unittest.cc
View file @ a5f20640
......@@ -46,30 +46,28 @@ enum { NO_FLAGS = 0x0000, HAS_MERGEABLE_TAILS = 0x0001, };
Instruction* SampleProgramOneInstruction(CodeGen* codegen, int* flags) {
// Create the most basic valid BPF program:
// RET ERR_ALLOWED
// RET 0
*flags = NO_FLAGS;
return codegen->MakeInstruction(BPF_RET + BPF_K,
ErrorCode(ErrorCode::ERR_ALLOWED));
return codegen->MakeInstruction(BPF_RET + BPF_K, 0);
}
Instruction* SampleProgramSimpleBranch(CodeGen* codegen, int* flags) {
// Create a program with a single branch:
// JUMP if eq 42 then $0 else $1
// 0: RET EPERM
// 1: RET ERR_ALLOWED
// 0: RET 1
// 1: RET 0
*flags = NO_FLAGS;
return codegen->MakeInstruction(
BPF_JMP + BPF_JEQ + BPF_K,
42,
codegen->MakeInstruction(BPF_RET + BPF_K, ErrorCode(EPERM)),
codegen->MakeInstruction(BPF_RET + BPF_K,
ErrorCode(ErrorCode::ERR_ALLOWED)));
codegen->MakeInstruction(BPF_RET + BPF_K, 1),
codegen->MakeInstruction(BPF_RET + BPF_K, 0));
}
Instruction* SampleProgramAtypicalBranch(CodeGen* codegen, int* flags) {
// Create a program with a single branch:
// JUMP if eq 42 then $0 else $0
// 0: RET ERR_ALLOWED
// 0: RET 0
// N.B.: As the instructions in both sides of the branch are already
// the same object, we do not actually have any "mergeable" branches.
......@@ -77,7 +75,7 @@ Instruction* SampleProgramAtypicalBranch(CodeGen* codegen, int* flags) {
*flags = NO_FLAGS;
Instruction* ret = codegen->MakeInstruction(
BPF_RET + BPF_K, ErrorCode(ErrorCode::ERR_ALLOWED));
BPF_RET + BPF_K, 0);
return codegen->MakeInstruction(BPF_JMP + BPF_JEQ + BPF_K, 42, ret, ret);
}
......@@ -88,9 +86,9 @@ Instruction* SampleProgramComplex(CodeGen* codegen, int* flags) {
// 1: JUMP if eq 42 then $2 else $4 (insn4)
// 2: JUMP to $3 (insn1)
// 3: LD 42 (insn0)
// RET ErrorCode(42) (insn2)
// RET 42 (insn2)
// 4: LD 42 (insn3)
// RET ErrorCode(42) (insn3+)
// RET 42 (insn3+)
*flags = HAS_MERGEABLE_TAILS;
Instruction* insn0 = codegen->MakeInstruction(BPF_LD + BPF_W + BPF_ABS, 42);
......@@ -104,7 +102,7 @@ Instruction* SampleProgramComplex(CodeGen* codegen, int* flags) {
SANDBOX_ASSERT(insn1->code == BPF_JMP + BPF_JA);
SANDBOX_ASSERT(insn1->jt_ptr == insn0);
Instruction* insn2 = codegen->MakeInstruction(BPF_RET + BPF_K, ErrorCode(42));
Instruction* insn2 = codegen->MakeInstruction(BPF_RET + BPF_K, 42);
SANDBOX_ASSERT(insn2);
SANDBOX_ASSERT(insn2->code == BPF_RET + BPF_K);
SANDBOX_ASSERT(insn2->next == NULL);
......@@ -114,7 +112,7 @@ Instruction* SampleProgramComplex(CodeGen* codegen, int* flags) {
Instruction* insn3 = codegen->MakeInstruction(
BPF_LD + BPF_W + BPF_ABS,
42,
codegen->MakeInstruction(BPF_RET + BPF_K, ErrorCode(42)));
codegen->MakeInstruction(BPF_RET + BPF_K, 42));
Instruction* insn4 =
codegen->MakeInstruction(BPF_JMP + BPF_JEQ + BPF_K, 42, insn1, insn3);
......@@ -162,12 +160,12 @@ Instruction* SampleProgramConfusingTails(CodeGen* codegen, int* flags) {
// 3) if A == 0x2; then JMP 4 else JMP 5
// 4) LOAD 0 // System call number
// 5) if A == 0x1; then JMP 6 else JMP 7
// 6) RET 0x50000 // errno = 0
// 7) RET 0x50001 // errno = 1
// 6) RET 0
// 7) RET 1
*flags = NO_FLAGS;
Instruction* i7 = codegen->MakeInstruction(BPF_RET, ErrorCode(1));
Instruction* i6 = codegen->MakeInstruction(BPF_RET, ErrorCode(0));
Instruction* i7 = codegen->MakeInstruction(BPF_RET + BPF_K, 1);
Instruction* i6 = codegen->MakeInstruction(BPF_RET + BPF_K, 0);
Instruction* i5 =
codegen->MakeInstruction(BPF_JMP + BPF_JEQ + BPF_K, 1, i6, i7);
Instruction* i4 = codegen->MakeInstruction(BPF_LD + BPF_W + BPF_ABS, 0, i5);
......@@ -191,10 +189,10 @@ Instruction* SampleProgramConfusingTailsBasic(CodeGen* codegen, int* flags) {
// 2) LOAD 0 // System call number
// 3) if A == 0x2; then JMP 4 else JMP 5
// 4) LOAD 0 // System call number
// 5) RET 0x50001 // errno = 1
// 5) RET 1
*flags = NO_FLAGS;
Instruction* i5 = codegen->MakeInstruction(BPF_RET, ErrorCode(1));
Instruction* i5 = codegen->MakeInstruction(BPF_RET + BPF_K, 1);
Instruction* i4 = codegen->MakeInstruction(BPF_LD + BPF_W + BPF_ABS, 0, i5);
Instruction* i3 =
codegen->MakeInstruction(BPF_JMP + BPF_JEQ + BPF_K, 2, i4, i5);
......@@ -216,22 +214,22 @@ Instruction* SampleProgramConfusingTailsMergeable(CodeGen* codegen,
//
// 0) LOAD 1 // ???
// 1) if A == 0x1; then JMP 2 else JMP 3
// 2) RET 0x5002a // errno = 42
// 2) RET 42
// 3) if A == 0x2; then JMP 4 else JMP 5
// 4) RET 0x5002a // errno = 42
// 4) RET 42
// 5) if A == 0x1; then JMP 6 else JMP 7
// 6) RET 0x50000 // errno = 0
// 7) RET 0x50001 // errno = 1
// 6) RET 0
// 7) RET 1
*flags = HAS_MERGEABLE_TAILS;
Instruction* i7 = codegen->MakeInstruction(BPF_RET, ErrorCode(1));
Instruction* i6 = codegen->MakeInstruction(BPF_RET, ErrorCode(0));
Instruction* i7 = codegen->MakeInstruction(BPF_RET + BPF_K, 1);
Instruction* i6 = codegen->MakeInstruction(BPF_RET + BPF_K, 0);
Instruction* i5 =
codegen->MakeInstruction(BPF_JMP + BPF_JEQ + BPF_K, 1, i6, i7);
Instruction* i4 = codegen->MakeInstruction(BPF_RET, ErrorCode(42));
Instruction* i4 = codegen->MakeInstruction(BPF_RET + BPF_K, 42);
Instruction* i3 =
codegen->MakeInstruction(BPF_JMP + BPF_JEQ + BPF_K, 2, i4, i5);
Instruction* i2 = codegen->MakeInstruction(BPF_RET, ErrorCode(42));
Instruction* i2 = codegen->MakeInstruction(BPF_RET + BPF_K, 42);
Instruction* i1 =
codegen->MakeInstruction(BPF_JMP + BPF_JEQ + BPF_K, 1, i2, i3);
Instruction* i0 = codegen->MakeInstruction(BPF_LD + BPF_W + BPF_ABS, 1, i1);
......
sandbox/linux/seccomp-bpf/sandbox_bpf.cc
View file @ a5f20640
......@@ -888,10 +888,14 @@ Instruction* SandboxBPF::AssembleJumpTable(CodeGen* gen,
}
Instruction* SandboxBPF::RetExpression(CodeGen* gen, const ErrorCode& err) {
if (err.error_type_ == ErrorCode::ET_COND) {
switch (err.error_type()) {
case ErrorCode::ET_COND:
return CondExpression(gen, err);
} else {
return gen->MakeInstruction(BPF_RET + BPF_K, err);
case ErrorCode::ET_SIMPLE:
case ErrorCode::ET_TRAP:
return gen->MakeInstruction(BPF_RET + BPF_K, err.err());
default:
SANDBOX_DIE("ErrorCode is not suitable for returning from a BPF program");
}
}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment