Chromium Code Reviews| Index: sandbox/linux/seccomp-bpf/sandbox_bpf.cc |
| diff --git a/sandbox/linux/seccomp-bpf/sandbox_bpf.cc b/sandbox/linux/seccomp-bpf/sandbox_bpf.cc |
| index f7c85113db789ec1676bd967034a83f372413e45..b38af571e27f334f5654c4643617c9f8ce05bc75 100644 |
| --- a/sandbox/linux/seccomp-bpf/sandbox_bpf.cc |
| +++ b/sandbox/linux/seccomp-bpf/sandbox_bpf.cc |
| @@ -5,6 +5,7 @@ |
| #include <time.h> |
| #include "sandbox/linux/seccomp-bpf/sandbox_bpf.h" |
| +#include "sandbox/linux/seccomp-bpf/syscall_iterator.h" |
| #include "sandbox/linux/seccomp-bpf/verifier.h" |
| // The kernel gives us a sandbox, we turn it into a playground :-) |
| @@ -34,11 +35,20 @@ void Sandbox::probeProcess(void) { |
| } |
| } |
| -ErrorCode Sandbox::allowAllEvaluator(int signo) { |
| - if (signo < static_cast<int>(MIN_SYSCALL) || |
| - signo > static_cast<int>(MAX_SYSCALL)) { |
| +bool Sandbox::isArmPrivateSyscall(int sysnum) { |
| + return SyscallIterator::IsArmPrivate(sysnum); |
| +} |
| + |
| +ErrorCode Sandbox::allowAllEvaluator(int sysnum) { |
| + if (sysnum < static_cast<int>(MIN_SYSCALL) || |
| + sysnum > static_cast<int>(MAX_SYSCALL)) { |
| return ErrorCode(ENOSYS); |
| } |
| +#if defined(__arm__) |
| + if (!isArmPrivateSyscall(sysnum)) { |
| + return ErrorCode(ENOSYS); |
| + } |
| +#endif |
| return ErrorCode(ErrorCode::ERR_ALLOWED); |
| } |
| @@ -272,46 +282,13 @@ bool Sandbox::isDenied(const ErrorCode& code) { |
| void Sandbox::policySanityChecks(EvaluateSyscall syscallEvaluator, |
| EvaluateArguments) { |
| - // Do some sanity checks on the policy. This will warn users if they do |
| - // things that are likely unsafe and unintended. |
| - // We also have similar checks later, when we actually compile the BPF |
| - // program. That catches problems with incorrectly stacked evaluators. |
| - if (!isDenied(syscallEvaluator(-1))) { |
| - SANDBOX_DIE("Negative system calls should always be disallowed by policy"); |
| - } |
| -#ifndef NDEBUG |
| -#if defined(__i386__) || defined(__x86_64__) |
| -#if defined(__x86_64__) && defined(__ILP32__) |
| - for (unsigned int sysnum = MIN_SYSCALL & ~0x40000000u; |
| - sysnum <= (MAX_SYSCALL & ~0x40000000u); |
| - ++sysnum) { |
| + for (SyscallIterator iter(true); !iter.Done(); ) { |
|
jln (very slow on Chromium)
2012/10/11 22:42:00
We still have the same problem as before on ARM.
W
Jorge Lucangeli Obes
2012/10/12 17:58:23
Confirmed that we don't.
|
| + uint32_t sysnum = iter.Next(); |
| if (!isDenied(syscallEvaluator(sysnum))) { |
| - SANDBOX_DIE("In x32 mode, you should not allow any non-x32 " |
| - "system calls"); |
| + SANDBOX_DIE("Policies should deny system calls that are outside the " |
| + "expected range (typically MIN_SYSCALL..MAX_SYSCALL)"); |
| } |
| } |
| -#else |
| - for (unsigned int sysnum = MIN_SYSCALL | 0x40000000u; |
| - sysnum <= (MAX_SYSCALL | 0x40000000u); |
| - ++sysnum) { |
| - if (!isDenied(syscallEvaluator(sysnum))) { |
| - SANDBOX_DIE("x32 system calls should be explicitly disallowed"); |
| - } |
| - } |
| -#endif |
| -#endif |
| -#endif |
| - // Check interesting boundary values just outside of the valid system call |
| - // range: 0x7FFFFFFF, 0x80000000, 0xFFFFFFFF, MIN_SYSCALL-1, MAX_SYSCALL+1. |
| - // They all should be denied. |
| - if (!isDenied(syscallEvaluator(std::numeric_limits<int>::max())) || |
| - !isDenied(syscallEvaluator(std::numeric_limits<int>::min())) || |
| - !isDenied(syscallEvaluator(-1)) || |
| - !isDenied(syscallEvaluator(static_cast<int>(MIN_SYSCALL) - 1)) || |
| - !isDenied(syscallEvaluator(static_cast<int>(MAX_SYSCALL) + 1))) { |
| - SANDBOX_DIE("Even for default-allow policies, you must never allow system " |
| - "calls outside of the standard system call range"); |
| - } |
| return; |
| } |
| @@ -464,32 +441,23 @@ void Sandbox::findRanges(Ranges *ranges) { |
| EvaluateSyscall evaluateSyscall = evaluators_.begin()->first; |
| uint32_t oldSysnum = 0; |
| ErrorCode oldErr = evaluateSyscall(oldSysnum); |
| - for (uint32_t sysnum = std::max(1u, MIN_SYSCALL); |
| - sysnum <= MAX_SYSCALL + 1; |
| - ++sysnum) { |
| + ErrorCode invalidErr = evaluateSyscall(MIN_SYSCALL-1); |
| + for (SyscallIterator iter(false); !iter.Done(); ) { |
| + uint32_t sysnum = iter.Next(); |
| ErrorCode err = evaluateSyscall(static_cast<int>(sysnum)); |
| - if (!err.Equals(oldErr)) { |
| + if (!iter.IsValid(sysnum) && !invalidErr.Equals(err)) { |
| + // A proper sandbox policy should always treat system calls outside of |
| + // the range MIN_SYSCALL..MAX_SYSCALL (i.e. anything that returns |
| + // "false" for SyscallIterator::IsValid()) identically. Typically, all |
| + // of these system calls would be denied with the same ErrorCode. |
| + SANDBOX_DIE("Invalid seccomp policy"); |
| + } |
| + if (!err.Equals(oldErr) || iter.Done()) { |
| ranges->push_back(Range(oldSysnum, sysnum-1, oldErr)); |
| oldSysnum = sysnum; |
| oldErr = err; |
| } |
| } |
| - |
| - // As we looped all the way past the valid system calls (i.e. MAX_SYSCALL+1), |
| - // "oldErr" should at this point be the "default" policy for all system call |
| - // numbers that don't have an explicit handler in the system call evaluator. |
| - // But as we are quite paranoid, we perform some more sanity checks to verify |
| - // that there actually is a consistent "default" policy in the first place. |
| - // We don't actually iterate over all possible 2^32 values, though. We just |
| - // perform spot checks at the boundaries. |
| - // The cases that we test are: 0x7FFFFFFF, 0x80000000, 0xFFFFFFFF. |
| - if (!oldErr.Equals(evaluateSyscall(std::numeric_limits<int>::max())) || |
| - !oldErr.Equals(evaluateSyscall(std::numeric_limits<int>::min())) || |
| - !oldErr.Equals(evaluateSyscall(-1))) { |
| - SANDBOX_DIE("Invalid seccomp policy"); |
| - } |
| - ranges->push_back( |
| - Range(oldSysnum, std::numeric_limits<unsigned>::max(), oldErr)); |
| } |
| void Sandbox::emitJumpStatements(Program *program, RetInsns *rets, |