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

Unified Diff: sandbox/linux/seccomp-bpf/sandbox_bpf.h

Issue 10833044: Refactored ErrorCode into it's own class (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Made changes requested by reviewer (jln) Created 8 years, 5 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
Index: sandbox/linux/seccomp-bpf/sandbox_bpf.h
diff --git a/sandbox/linux/seccomp-bpf/sandbox_bpf.h b/sandbox/linux/seccomp-bpf/sandbox_bpf.h
index 414327de1df8400c20fa2cacbb5c21f1994738f4..ee8deddab054016b4118e3a24061bb179de9b1df 100644
--- a/sandbox/linux/seccomp-bpf/sandbox_bpf.h
+++ b/sandbox/linux/seccomp-bpf/sandbox_bpf.h
@@ -45,6 +45,15 @@
#include "base/logging.h"
#endif
+#ifdef SECCOMP_BPF_VALGRIND_HACKS
jln (very slow on Chromium) 2012/07/30 18:50:39 In theory this should be #if defined(SECCOMP....).
+#ifndef SECCOMP_BPF_STANDALONE
+#include "base/third_party/valgrind/valgrind.h"
+#endif
+#endif
+
+#include "sandbox/linux/seccomp-bpf/errorcode.h"
+
+
// The Seccomp2 kernel ABI is not part of older versions of glibc.
// As we can't break compilation with these versions of the library,
// we explicitly define all missing symbols.
@@ -109,6 +118,9 @@
#error Unsupported target platform
#endif
+
+namespace playground2 {
+
struct arch_seccomp_data {
int nr;
uint32_t arch;
@@ -131,9 +143,6 @@ struct arch_sigsys {
void operator=(const TypeName&)
#endif
-
-namespace playground2 {
-
class Sandbox {
public:
enum SandboxStatus {
@@ -144,17 +153,6 @@ class Sandbox {
STATUS_ENABLED // The sandbox is now active
};
- enum {
- SB_INVALID = -1,
- SB_ALLOWED = 0x0000,
- SB_INSPECT_ARG_1 = 0x8001,
- SB_INSPECT_ARG_2 = 0x8002,
- SB_INSPECT_ARG_3 = 0x8004,
- SB_INSPECT_ARG_4 = 0x8008,
- SB_INSPECT_ARG_5 = 0x8010,
- SB_INSPECT_ARG_6 = 0x8020
- };
-
// TrapFnc is a pointer to a function that handles Seccomp traps in
// user-space. The seccomp policy can request that a trap handler gets
// installed; it does so by returning a suitable ErrorCode() from the
@@ -165,67 +163,6 @@ class Sandbox {
// http://pubs.opengroup.org/onlinepubs/009695399/functions/xsh_chap02_04.html
typedef intptr_t (*TrapFnc)(const struct arch_seccomp_data& args, void *aux);
- class ErrorCode {
- friend class Sandbox;
- public:
- // We can either wrap a symbolic ErrorCode (i.e. enum values), an errno
- // value (in the range 1..4095), or a pointer to a TrapFnc callback
- // handling a SECCOMP_RET_TRAP trap.
- // All of these different values are stored in the "err_" field. So, code
- // that is using the ErrorCode class typically operates on a single 32bit
- // field.
- // This is not only quiet efficient, it also makes the API really easy to
- // use.
- ErrorCode(int err = SB_INVALID)
- : id_(0),
- fnc_(NULL),
- aux_(NULL) {
- switch (err) {
- case SB_INVALID:
- err_ = SECCOMP_RET_INVALID;
- break;
- case SB_ALLOWED:
- err_ = SECCOMP_RET_ALLOW;
- break;
- case SB_INSPECT_ARG_1...SB_INSPECT_ARG_6:
- die("Not implemented");
- break;
- case 1 ... 4095:
- err_ = SECCOMP_RET_ERRNO + err;
- break;
- default:
- die("Invalid use of ErrorCode object");
- }
- }
-
- // 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.
- // The getTrapId() function assigns one unique id (starting at 1) for
- // each distinct pair of TrapFnc and auxiliary data.
- ErrorCode(TrapFnc fnc, const void *aux, int id = 0) :
- id_(id ? id : getTrapId(fnc, aux)),
- fnc_(fnc),
- aux_(const_cast<void *>(aux)),
- err_(SECCOMP_RET_TRAP + id_) {
- }
-
- // Destructor doesn't need to do anything.
- ~ErrorCode() { }
-
- // Always return the value that goes into the BPF filter program.
- operator uint32_t() const { return err_; }
-
- protected:
- // Fields needed for SECCOMP_RET_TRAP callbacks
- int id_;
- TrapFnc fnc_;
- void *aux_;
-
- // 32bit field used for all possible types of ErrorCode values
- uint32_t err_;
- };
-
enum Operation {
OP_NOP, OP_EQUAL, OP_NOTEQUAL, OP_LESS,
OP_LESS_EQUAL, OP_GREATER, OP_GREATER_EQUAL,
@@ -273,57 +210,25 @@ class Sandbox {
static void setSandboxPolicy(EvaluateSyscall syscallEvaluator,
EvaluateArguments argumentEvaluator);
+ // We can use ErrorCode to request calling of a trap handler. This method
+ // performs the required wrapping of the callback function into an
+ // ErrorCode object.
+ static ErrorCode Trap(ErrorCode::TrapFnc fnc, const void *aux);
+
+ // Kill the program and print an error message.
+ static ErrorCode Kill(const char *msg);
+
// This is the main public entry point. It finds all system calls that
// need rewriting, sets up the resources needed by the sandbox, and
// enters Seccomp mode.
static void startSandbox();
- protected:
- // Print an error message and terminate the program. Used for fatal errors.
- static void die(const char *msg) __attribute__((noreturn)) {
- if (msg) {
-#ifndef SECCOMP_BPF_STANDALONE
- if (!dryRun_) {
- // LOG(FATAL) is not neccessarily async-signal safe. It would be
- // better to always use the code for the SECCOMP_BPF_STANDALONE case.
- // But that prevents the logging and reporting infrastructure from
- // picking up sandbox related crashes.
- // For now, in picking between two evils, we decided in favor of
- // LOG(FATAL). In the long run, we probably want to rewrite this code
- // to be async-signal safe.
- LOG(FATAL) << msg;
- } else
-#endif
- {
- // If there is no logging infrastructure in place, we just write error
- // messages to stderr.
- // We also write to stderr, if we are called in a child process from
- // supportsSeccompSandbox(). This makes sure we can actually do the
- // correct logging from the parent process, which is more likely to
- // have access to logging infrastructure.
- if (HANDLE_EINTR(write(2, msg, strlen(msg)))) { }
- if (HANDLE_EINTR(write(2, "\n", 1))) { }
- }
- }
- for (;;) {
- // exit_group() should exit our program. After all, it is defined as a
- // function that doesn't return. But things can theoretically go wrong.
- // Especially, since we are dealing with system call filters. Continuing
- // execution would be very bad in most cases where die() gets called.
- // So, if there is no way for us to ask for the program to exit, the next
- // best thing we can do is to loop indefinitely. Maybe, somebody will
- // notice and file a bug...
- syscall(__NR_exit_group, 1);
- _exit(1);
- }
- }
-
- // Get a file descriptor pointing to "/proc", if currently available.
- static int getProcFd() { return proc_fd_; }
-
private:
+ friend class ErrorCode;
friend class Util;
friend class Verifier;
+
+
struct Range {
Range(uint32_t f, uint32_t t, const ErrorCode& e) :
from(f),
@@ -342,6 +247,7 @@ class Sandbox {
typedef std::vector<Range> Ranges;
typedef std::map<uint32_t, std::vector<FixUp> > RetInsns;
typedef std::vector<struct sock_filter> Program;
+ typedef std::map<uint32_t, ErrorCode> ErrMap;
typedef std::vector<ErrorCode> Traps;
typedef std::map<std::pair<TrapFnc, const void *>, int> TrapIds;
@@ -354,6 +260,7 @@ class Sandbox {
EvaluateSyscall syscallEvaluator,
int proc_fd);
static bool isSingleThreaded(int proc_fd);
+ static bool isDenied(const ErrorCode& code);
static bool disableFilesystem();
static void policySanityChecks(EvaluateSyscall syscallEvaluator,
EvaluateArguments argumentEvaluator);
@@ -367,10 +274,54 @@ class Sandbox {
static intptr_t bpfFailure(const struct arch_seccomp_data& data, void *aux);
static int getTrapId(TrapFnc fnc, const void *aux);
+ // Get a file descriptor pointing to "/proc", if currently available.
+ static int proc_fd() { return proc_fd_; }
+
+ // Print an error message and terminate the program. Used for fatal errors.
+ static void die(const char *msg) __attribute__((noreturn)) {
+ if (msg) {
+#ifndef SECCOMP_BPF_STANDALONE
+ if (!dryRun_) {
+ // LOG(FATAL) is not neccessarily async-signal safe. It would be
+ // better to always use the code for the SECCOMP_BPF_STANDALONE case.
+ // But that prevents the logging and reporting infrastructure from
+ // picking up sandbox related crashes.
+ // For now, in picking between two evils, we decided in favor of
+ // LOG(FATAL). In the long run, we probably want to rewrite this code
+ // to be async-signal safe.
+ LOG(FATAL) << msg;
+ } else
+#endif
+ {
+ // If there is no logging infrastructure in place, we just write error
+ // messages to stderr.
+ // We also write to stderr, if we are called in a child process from
+ // supportsSeccompSandbox(). This makes sure we can actually do the
+ // correct logging from the parent process, which is more likely to
+ // have access to logging infrastructure.
+ if (HANDLE_EINTR(write(2, msg, strlen(msg)))) { }
+ if (HANDLE_EINTR(write(2, "\n", 1))) { }
+ }
+ }
+ for (;;) {
+ // exit_group() should exit our program. After all, it is defined as a
+ // function that doesn't return. But things can theoretically go wrong.
+ // Especially, since we are dealing with system call filters. Continuing
+ // execution would be very bad in most cases where die() gets called.
+ // So, if there is no way for us to ask for the program to exit, the next
+ // best thing we can do is to loop indefinitely. Maybe, somebody will
+ // notice and file a bug...
+ syscall(__NR_exit_group, 1);
+ _exit(1);
+ }
+ }
+
+
static bool dryRun_;
static SandboxStatus status_;
static int proc_fd_;
static Evaluators evaluators_;
+ static ErrMap errMap_;
static Traps *traps_;
static TrapIds trapIds_;
static ErrorCode *trapArray_;

Powered by Google App Engine
This is Rietveld 408576698