Seccomp-BPF: add a new synthetic unittest

sandbox/linux/seccomp-bpf/sandbox_bpf_unittest.cc

Index: sandbox/linux/seccomp-bpf/sandbox_bpf_unittest.cc
diff --git a/sandbox/linux/seccomp-bpf/sandbox_bpf_unittest.cc b/sandbox/linux/seccomp-bpf/sandbox_bpf_unittest.cc
index a661c8d6b7ff6f4d3f5822ba410b9f0ff9865cc3..d01e9120cccb85090aebca366bcd1ae99aea4ce3 100644
--- a/sandbox/linux/seccomp-bpf/sandbox_bpf_unittest.cc
+++ b/sandbox/linux/seccomp-bpf/sandbox_bpf_unittest.cc
@@ -198,4 +198,56 @@ TEST(SandboxBpf, BasicBlacklistWithSigsys) {
   TryPolicyInProcess(BlacklistNanosleepPolicySigsys, NanosleepProcessSigsys);
 }
 
+// A more complex, but synthetic policy. This tests the correctness of the BPF
+// program by iterating through all syscalls and checking for a errno that

[palmer, 2012/06/28 19:58:14]

Nit: "for an errno" :)

[jln (very slow on Chromium), 2012/06/28 20:57:45]

Done.

+// depends on the syscall number. Unlike the Verifier, this exercises the BPF
+// interpreter in the kernel.
+
+int SysnoToRandomErrno(int sysno) {

[palmer, 2012/06/28 19:58:14]

I think this only makes sense if you have some context that I don't have. More
documentation? Is there some inherent relationship between contiguous system
call numbers?

[jln (very slow on Chromium), 2012/06/28 20:57:45]

Let me know if this is more clear.

+  // Small contiguous sets of 3 system calls return an errno equal to the
+  // index of that set + 1 (so that we never return a NUL errno).
+  return ((sysno & ~3) >> 2) % 29 + 1;
+}
+
+Sandbox::ErrorCode SyntheticPolicy(int sysno) {
+  if (sysno < static_cast<int>(MIN_SYSCALL) ||
+      sysno > static_cast<int>(MAX_SYSCALL)) {
+    // FIXME: we should really not have to do that in a trivial policy.
+    return ENOSYS;
+  }
+  if (sysno == __NR_exit_group)
+    // exit_group() is special, we really need it to work.
+    return Sandbox::SB_ALLOWED;
+  else {

[palmer, 2012/06/28 19:58:14]

Nit: Two-line (one code line) block gets no curly braces, but one-line block
does? I'd probably go with braces for both, for clarity and consistency.

[jln (very slow on Chromium), 2012/06/28 20:57:45]

Done.

+    return SysnoToRandomErrno(sysno);
+  }
+}
+
+void SyntheticProcess(void) {
+  // Ensure that that kExpectedReturnValue + syscallnumber + 1 does not int
+  // overflow.
+  if (std::numeric_limits<int>::max() - kExpectedReturnValue - 1 <
+      static_cast<int>(MAX_SYSCALL)) {
+    ExitGroup(1);
+  }
+  for (int syscall_number =  static_cast<int>(MIN_SYSCALL);

[palmer, 2012/06/28 19:58:14]

Why are these static_casts necessary? Can we just declare syscall_number/sysno
as the right type, whatever that is?

[jln (very slow on Chromium), 2012/06/28 20:57:45]

Perhaps surprisingly, syscalls are actually signed, however MIN_SYSCALL /
MAX_SYSCALL are unsigned integer, for a reason that I don't remember.

+           syscall_number <= static_cast<int>(MAX_SYSCALL);
+         ++syscall_number) {
+    if (syscall_number == __NR_exit_group) {
+      // exit_group() is special
+      continue;
+    }
+    errno = 0;
+    if (syscall(syscall_number) != -1 ||
+        errno != SysnoToRandomErrno(syscall_number)) {

[palmer, 2012/06/28 19:58:14]

I don't see why this is correct. Shouldn't we be expecting a specific errno
value?

[jln (very slow on Chromium), 2012/06/28 20:57:45]

This is what this is doing, isn't it ? I added a comment, hopefully it's more
clear.

+      ExitGroup(kExpectedReturnValue + syscall_number + 1);
+    }
+  }
+  ExitGroup(kExpectedReturnValue);
+}
+
+TEST(SandboxBpf, SyntheticPolicy) {
+  TryPolicyInProcess(SyntheticPolicy, SyntheticProcess);
+}
+
 } // namespace