Commit 046a627a authored by mdempsky's avatar mdempsky Committed by Commit bot

sandbox: Avoid ~(flag1 | flag2 | ...) expressions

BUG=416948

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

Cr-Commit-Position: refs/heads/master@{#296262}
parent cc5ed864
...@@ -61,9 +61,9 @@ ResultExpr RestrictFcntlCommands() { ...@@ -61,9 +61,9 @@ ResultExpr RestrictFcntlCommands() {
// libevent and SetNonBlocking. As the latter mix O_NONBLOCK to // libevent and SetNonBlocking. As the latter mix O_NONBLOCK to
// the return value of F_GETFL, so we need to allow O_ACCMODE in // the return value of F_GETFL, so we need to allow O_ACCMODE in
// addition to O_NONBLOCK. // addition to O_NONBLOCK.
const unsigned long denied_mask = ~(O_ACCMODE | O_NONBLOCK); const uint64_t kAllowedMask = O_ACCMODE | O_NONBLOCK;
return If((cmd == F_SETFD && long_arg == FD_CLOEXEC) || cmd == F_GETFL || return If((cmd == F_SETFD && long_arg == FD_CLOEXEC) || cmd == F_GETFL ||
(cmd == F_SETFL && (long_arg & denied_mask) == 0), (cmd == F_SETFL && (long_arg & ~kAllowedMask) == 0),
Allow()).Else(CrashSIGSYS()); Allow()).Else(CrashSIGSYS());
} }
...@@ -78,10 +78,9 @@ ResultExpr RestrictClone() { ...@@ -78,10 +78,9 @@ ResultExpr RestrictClone() {
ResultExpr RestrictFutexOperation() { ResultExpr RestrictFutexOperation() {
// TODO(hamaji): Allow only FUTEX_PRIVATE_FLAG futexes. // TODO(hamaji): Allow only FUTEX_PRIVATE_FLAG futexes.
const int kAllowedFutexFlags = FUTEX_PRIVATE_FLAG | FUTEX_CLOCK_REALTIME; const uint64_t kAllowedFutexFlags = FUTEX_PRIVATE_FLAG | FUTEX_CLOCK_REALTIME;
const int kOperationMask = ~kAllowedFutexFlags;
const Arg<int> op(1); const Arg<int> op(1);
return Switch(op & kOperationMask) return Switch(op & ~kAllowedFutexFlags)
.CASES((FUTEX_WAIT, .CASES((FUTEX_WAIT,
FUTEX_WAKE, FUTEX_WAKE,
FUTEX_REQUEUE, FUTEX_REQUEUE,
...@@ -113,20 +112,20 @@ ResultExpr RestrictSocketcall() { ...@@ -113,20 +112,20 @@ ResultExpr RestrictSocketcall() {
ResultExpr RestrictMprotect() { ResultExpr RestrictMprotect() {
// TODO(jln, keescook, drewry): Limit the use of mprotect by adding // TODO(jln, keescook, drewry): Limit the use of mprotect by adding
// some features to linux kernel. // some features to linux kernel.
const uint32_t denied_mask = ~(PROT_READ | PROT_WRITE | PROT_EXEC); const uint64_t kAllowedMask = PROT_READ | PROT_WRITE | PROT_EXEC;
const Arg<int> prot(2); const Arg<int> prot(2);
return If((prot & denied_mask) == 0, Allow()).Else(CrashSIGSYS()); return If((prot & ~kAllowedMask) == 0, Allow()).Else(CrashSIGSYS());
} }
ResultExpr RestrictMmap() { ResultExpr RestrictMmap() {
const uint32_t denied_flag_mask = ~(MAP_SHARED | MAP_PRIVATE | const uint64_t kAllowedFlagMask =
MAP_ANONYMOUS | MAP_STACK | MAP_FIXED); MAP_SHARED | MAP_PRIVATE | MAP_ANONYMOUS | MAP_STACK | MAP_FIXED;
// When PROT_EXEC is specified, IRT mmap of Non-SFI NaCl helper // When PROT_EXEC is specified, IRT mmap of Non-SFI NaCl helper
// calls mmap without PROT_EXEC and then adds PROT_EXEC by mprotect, // calls mmap without PROT_EXEC and then adds PROT_EXEC by mprotect,
// so we do not need to allow PROT_EXEC in mmap. // so we do not need to allow PROT_EXEC in mmap.
const uint32_t denied_prot_mask = ~(PROT_READ | PROT_WRITE); const uint64_t kAllowedProtMask = PROT_READ | PROT_WRITE;
const Arg<int> prot(2), flags(3); const Arg<int> prot(2), flags(3);
return If((prot & denied_prot_mask) == 0 && (flags & denied_flag_mask) == 0, return If((prot & ~kAllowedProtMask) == 0 && (flags & ~kAllowedFlagMask) == 0,
Allow()).Else(CrashSIGSYS()); Allow()).Else(CrashSIGSYS());
} }
......
...@@ -43,8 +43,8 @@ class SandboxBPF; ...@@ -43,8 +43,8 @@ class SandboxBPF;
// if (sysno == __NR_fcntl) { // if (sysno == __NR_fcntl) {
// Arg<int> fd(0), cmd(1); // Arg<int> fd(0), cmd(1);
// Arg<unsigned long> flags(2); // Arg<unsigned long> flags(2);
// const unsigned long kBadFlags = ~(O_ACCMODE | O_NONBLOCK); // const uint64_t kGoodFlags = O_ACCMODE | O_NONBLOCK;
// return If(fd == 0 && cmd == F_SETFL && (flags & kBadFlags) == 0, // return If(fd == 0 && cmd == F_SETFL && (flags & ~kGoodFlags) == 0,
// Allow()) // Allow())
// .ElseIf(cmd == F_DUPFD || cmd == F_DUPFD_CLOEXEC, // .ElseIf(cmd == F_DUPFD || cmd == F_DUPFD_CLOEXEC,
// Error(EMFILE)) // Error(EMFILE))
......
...@@ -142,11 +142,11 @@ ResultExpr RestrictMmapFlags() { ...@@ -142,11 +142,11 @@ ResultExpr RestrictMmapFlags() {
// Significantly, we don't permit MAP_HUGETLB, or the newer flags such as // Significantly, we don't permit MAP_HUGETLB, or the newer flags such as
// MAP_POPULATE. // MAP_POPULATE.
// TODO(davidung), remove MAP_DENYWRITE with updated Tegra libraries. // TODO(davidung), remove MAP_DENYWRITE with updated Tegra libraries.
const uint32_t denied_mask = const uint64_t kAllowedMask = MAP_SHARED | MAP_PRIVATE | MAP_ANONYMOUS |
~(MAP_SHARED | MAP_PRIVATE | MAP_ANONYMOUS | MAP_STACK | MAP_NORESERVE | MAP_STACK | MAP_NORESERVE | MAP_FIXED |
MAP_FIXED | MAP_DENYWRITE); MAP_DENYWRITE;
const Arg<int> flags(3); const Arg<int> flags(3);
return If((flags & denied_mask) == 0, Allow()).Else(CrashSIGSYS()); return If((flags & ~kAllowedMask) == 0, Allow()).Else(CrashSIGSYS());
} }
ResultExpr RestrictMprotectFlags() { ResultExpr RestrictMprotectFlags() {
...@@ -154,9 +154,9 @@ ResultExpr RestrictMprotectFlags() { ...@@ -154,9 +154,9 @@ ResultExpr RestrictMprotectFlags() {
// "denied" mask because of the negation operator. // "denied" mask because of the negation operator.
// Significantly, we don't permit weird undocumented flags such as // Significantly, we don't permit weird undocumented flags such as
// PROT_GROWSDOWN. // PROT_GROWSDOWN.
const uint32_t denied_mask = ~(PROT_READ | PROT_WRITE | PROT_EXEC); const uint64_t kAllowedMask = PROT_READ | PROT_WRITE | PROT_EXEC;
const Arg<int> prot(2); const Arg<int> prot(2);
return If((prot & denied_mask) == 0, Allow()).Else(CrashSIGSYS()); return If((prot & ~kAllowedMask) == 0, Allow()).Else(CrashSIGSYS());
} }
ResultExpr RestrictFcntlCommands() { ResultExpr RestrictFcntlCommands() {
...@@ -165,15 +165,15 @@ ResultExpr RestrictFcntlCommands() { ...@@ -165,15 +165,15 @@ ResultExpr RestrictFcntlCommands() {
// allowed ones, and the variable is a "denied" mask because of the negation // allowed ones, and the variable is a "denied" mask because of the negation
// operator. // operator.
// Glibc overrides the kernel's O_LARGEFILE value. Account for this. // Glibc overrides the kernel's O_LARGEFILE value. Account for this.
int kOLargeFileFlag = O_LARGEFILE; uint64_t kOLargeFileFlag = O_LARGEFILE;
if (IsArchitectureX86_64() || IsArchitectureI386() || IsArchitectureMips()) if (IsArchitectureX86_64() || IsArchitectureI386() || IsArchitectureMips())
kOLargeFileFlag = 0100000; kOLargeFileFlag = 0100000;
const Arg<int> cmd(1); const Arg<int> cmd(1);
const Arg<long> long_arg(2); const Arg<long> long_arg(2);
unsigned long denied_mask = ~(O_ACCMODE | O_APPEND | O_NONBLOCK | O_SYNC | const uint64_t kAllowedMask = O_ACCMODE | O_APPEND | O_NONBLOCK | O_SYNC |
kOLargeFileFlag | O_CLOEXEC | O_NOATIME); kOLargeFileFlag | O_CLOEXEC | O_NOATIME;
return Switch(cmd) return Switch(cmd)
.CASES((F_GETFL, .CASES((F_GETFL,
F_GETFD, F_GETFD,
...@@ -185,7 +185,7 @@ ResultExpr RestrictFcntlCommands() { ...@@ -185,7 +185,7 @@ ResultExpr RestrictFcntlCommands() {
F_DUPFD_CLOEXEC), F_DUPFD_CLOEXEC),
Allow()) Allow())
.Case(F_SETFL, .Case(F_SETFL,
If((long_arg & denied_mask) == 0, Allow()).Else(CrashSIGSYS())) If((long_arg & ~kAllowedMask) == 0, Allow()).Else(CrashSIGSYS()))
.Default(CrashSIGSYS()); .Default(CrashSIGSYS());
} }
...@@ -226,10 +226,9 @@ ResultExpr RestrictKillTarget(pid_t target_pid, int sysno) { ...@@ -226,10 +226,9 @@ ResultExpr RestrictKillTarget(pid_t target_pid, int sysno) {
} }
ResultExpr RestrictFutex() { ResultExpr RestrictFutex() {
const int kAllowedFutexFlags = FUTEX_PRIVATE_FLAG | FUTEX_CLOCK_REALTIME; const uint64_t kAllowedFutexFlags = FUTEX_PRIVATE_FLAG | FUTEX_CLOCK_REALTIME;
const int kOperationMask = ~kAllowedFutexFlags;
const Arg<int> op(1); const Arg<int> op(1);
return Switch(op & kOperationMask) return Switch(op & ~kAllowedFutexFlags)
.CASES((FUTEX_WAIT, .CASES((FUTEX_WAIT,
FUTEX_WAKE, FUTEX_WAKE,
FUTEX_REQUEUE, FUTEX_REQUEUE,
......
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