Explicitly test bit 30 in the system call number to distinguish between the new x32 API and older I…

sandbox/linux/seccomp-bpf/sandbox_bpf.cc

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 a60b2080827704c9ffeaef8dff00c7cfb7b7a866..f07327fa320ad039d5f04cb4317404804d9ef288 100644
--- a/sandbox/linux/seccomp-bpf/sandbox_bpf.cc
+++ b/sandbox/linux/seccomp-bpf/sandbox_bpf.cc
@@ -215,8 +215,7 @@ void Sandbox::installFilter() {
   // system call.
   std::vector<struct sock_filter> program;
   program.push_back((struct sock_filter)
-                    BPF_STMT(BPF_LD+BPF_W+BPF_ABS,
-                             offsetof(struct arch_seccomp_data, arch)));
+    BPF_STMT(BPF_LD+BPF_W+BPF_ABS, offsetof(struct arch_seccomp_data, arch)));
   program.push_back((struct sock_filter)
     BPF_JUMP(BPF_JMP+BPF_JEQ+BPF_K, SECCOMP_ARCH, 1, 0));
   program.push_back((struct sock_filter)
@@ -226,6 +225,21 @@ void Sandbox::installFilter() {
   program.push_back((struct sock_filter)
     BPF_STMT(BPF_LD+BPF_W+BPF_ABS, offsetof(struct arch_seccomp_data, nr)));
 
+  // On Intel architectures, verify that system call numbers are in the
+  // expected number range. The older i386 and x86-64 APIs clear bit 30
+  // on all system calls. The newer x86-32 API always sets bit 30.
+#if defined(__i386__) || defined(__x86_64__)

[Chris Evans, 2012/06/07 01:02:38]

I don't quite get this bit. If we're compiling for __i386__, why do anything? I
probably don't understand x86-32, but doesn't it only apply to 64-bit kernels?

[Markus (顧孟勤), 2012/06/07 01:33:28]

The preprocessor tests check for the expected API that our program is using.
They have nothing to do with what the kernel actually supports.

And any 64bit kernel allows programs to use both i386 and x86-64 system calls;
no matter whether the program happens to be 32bit or 64bit.

So, I really don't want to rely on the kernel disabling the x32 API when the
program happens to be compiled as a i386 32bit binary. That probably isn't
happening in current kernels.

In an ideal world, the test for the SECCOMP_ARCH already takes care of all of
this. But I can't find any guarantee that this is always going to be the case
(in fact, there I am not even sure there is an AUDIT_ARCH_X32). And there have
been bugs in this part of the kernel before.

+#if defined(__x86_64__) && defined(__ILP32__)
+  program.push_back((struct sock_filter)
+    BPF_JUMP(BPF_JMP+BPF_JSET+BPF_K, 0x40000000, 1, 0));
+#else
+  program.push_back((struct sock_filter)
+    BPF_JUMP(BPF_JMP+BPF_JSET+BPF_K, 0x40000000, 0, 1));
+#endif
+  program.push_back((struct sock_filter)
+    BPF_STMT(BPF_RET+BPF_K, SECCOMP_RET_ERRNO + SECCOMP_DENY_ERRNO));

[Chris Evans, 2012/06/07 01:02:38]

If we get here, seems like something extraordinarily bad has happened.
SECCOMP_RET_KILL IMHO

[Markus (顧孟勤), 2012/06/07 01:33:28]

Let me know, what you prefer until we get a better SIGSYS handler in place.
There are downsides to killing the program, but since we don't really have users
at this very moment, it probably doesn't matter much.

If I don't hear from you, I'll just upload a new CL with SECCOMP_RET_KILL and a
comment that this should eventually become SECCOMP_RET_TRAP.

+#endif
+
   // Evaluate all possible system calls and depending on their
   // exit codes generate a BPF filter.
   // This is very inefficient right now. We need to be much smarter