Seccomp: merge i386 and x86_64 architectures in system call sets.

content/common/sandbox_seccomp_bpf_linux.cc

Index: content/common/sandbox_seccomp_bpf_linux.cc
diff --git a/content/common/sandbox_seccomp_bpf_linux.cc b/content/common/sandbox_seccomp_bpf_linux.cc
index 5a97ea7ea11a1d5d92538772c0d8830f35c6b284..0d138693e800ed863739dd4e1278b9ff21296628 100644
--- a/content/common/sandbox_seccomp_bpf_linux.cc
+++ b/content/common/sandbox_seccomp_bpf_linux.cc
@@ -147,7 +147,7 @@ intptr_t GpuOpenSIGSYS_Handler(const struct arch_seccomp_data& args,
   }
 }
 
-#if defined(__x86_64__)
+#if defined(__i386__) || defined(__x86_64__)
 
 // The functions below cover all existing x86_64 system calls.
 // The implicitly defined sets form a partition of the sets of
@@ -176,7 +176,13 @@ bool IsAllowedGettime(int sysno) {
     case __NR_clock_getres:     // Could be allowed.
     case __NR_clock_nanosleep:  // Could be allowed.
     case __NR_clock_settime:    // Privileged.
+#if defined(__i386__)

[Markus (顧孟勤), 2012/08/11 00:58:40]

I would generally prefer if you did something like:

#if defined(__NR_ftime)

instead of

#if defined(__i386__)

Feel free to still leave a comment in there saying that this symbol is only
defined on __i386__ (and possibly others).

If we do this right, we should have a much easier time porting to things like
ARM.

There are a good number of subtle changes between system call lists. But on a
per-system-call basis, there are usually very few different options. Typically,
either the system call exists or it doesn't.

It should ultimately be possible to come up with policies that work correctly
for all Linux platforms.

[jln (very slow on Chromium), 2012/08/11 01:36:38]

In general that seems like a good idea. In practice I'm not 100% convinced yet,
because we need to develop and test policies on specific platform, and it's
really very useful to immediately see that this or is not available on i386.
Yeah comments can partly solve it I suppose.

But more importantly: there are big blocks (like the set*id32) where having one
big #if defined(__i386__) is super useful. In the policies themselves I suspect
that being able to regroup a bunch of things under if defined(__i386__) will be
nice.

Maybe it's something we'll want to consider a bit later when we're more generic
and abstracted? I really hope we can get to a point where we have an API to
start a sandbox that takes a few options where you state your intentions and
that's all we need.

Also it makes things a bit fragile in case of some bug in how we include
headers, but that could be solved via tests and a few carefully crafted #error.

What do you think?

+    case __NR_ftime:            // Obsolete.
+#endif
     case __NR_settimeofday:     // Privileged.
+#if defined(__i386__)
+    case __NR_stime:
+#endif
     default:
       return false;
   }
@@ -211,23 +217,36 @@ bool IsFileSystem(int sysno) {
     case __NR_access:          // EPERM not a valid errno.
     case __NR_chmod:
     case __NR_chown:
+#if defined(__i386__)
+    case __NR_chown32:
+#endif
     case __NR_creat:
     case __NR_execve:
     case __NR_faccessat:       // EPERM not a valid errno.
     case __NR_fchmodat:
     case __NR_fchownat:        // Should be called chownat ?
+#if defined(__x86_64__)
+    case __NR_newfstatat:      // fstatat(). EPERM not a valid errno.
+#elif defined(__i386__)
+    case __NR_fstatat64:
+#endif
     case __NR_futimesat:       // Should be called utimesat ?
     case __NR_lchown:
+#if defined(__i386__)
+    case __NR_lchown32:
+#endif
     case __NR_link:
     case __NR_linkat:
     case __NR_lookup_dcookie:  // ENOENT not a valid errno.
     case __NR_lstat:           // EPERM not a valid errno.
+#if defined(__i386__)
+    case __NR_oldlstat:
+    case __NR_lstat64:
+#endif
     case __NR_mkdir:
     case __NR_mkdirat:
     case __NR_mknod:
     case __NR_mknodat:
-    case __NR_newfstatat:      // EPERM not a valid errno.
-                               // Should be called statat ?
     case __NR_open:
     case __NR_openat:
     case __NR_readlink:        // EPERM not a valid errno.
@@ -236,10 +255,20 @@ bool IsFileSystem(int sysno) {
     case __NR_renameat:
     case __NR_rmdir:
     case __NR_stat:            // EPERM not a valid errno.
+#if defined(__i386__)
+    case __NR_oldstat:
+    case __NR_stat64:
+#endif
     case __NR_statfs:          // EPERM not a valid errno.
+#if defined(__i386__)
+    case __NR_statfs64:
+#endif
     case __NR_symlink:
     case __NR_symlinkat:
     case __NR_truncate:
+#if defined(__i386__)
+    case __NR_truncate64:
+#endif
     case __NR_unlink:
     case __NR_unlinkat:
     case __NR_uselib:          // Neither EPERM, nor ENOENT are valid errno.
@@ -256,13 +285,25 @@ bool IsFileSystem(int sysno) {
 bool IsAllowedFileSystemAccessViaFd(int sysno) {
   switch (sysno) {
     case __NR_fstat:
+#if defined(__i386__)
+    case __NR_fstat64:
+#endif
       return true;
     // TODO(jln): these should be denied gracefully as well (moved below).
     case __NR_fadvise64:        // EPERM not a valid errno.
+#if defined(__i386__)
+    case __NR_fadvise64_64:
+#endif
     case __NR_fdatasync:        // EPERM not a valid errno.
     case __NR_flock:            // EPERM not a valid errno.
     case __NR_fstatfs:          // Give information about the whole filesystem.
+#if defined(__i386__)
+    case __NR_fstatfs64:
+#endif
     case __NR_fsync:            // EPERM not a valid errno.
+#if defined(__i386__)
+    case __NR_oldfstat:
+#endif
     case __NR_sync_file_range:  // EPERM not a valid errno.
     default:
       return false;
@@ -275,9 +316,18 @@ bool IsDeniedFileSystemAccessViaFd(int sysno) {
     case __NR_fallocate:
     case __NR_fchmod:
     case __NR_fchown:
+#if defined(__i386__)
+    case __NR_fchown32:
+#endif
     case __NR_ftruncate:
+#if defined(__i386__)
+    case __NR_ftruncate64:
+#endif
     case __NR_getdents:         // EPERM not a valid errno.
     case __NR_getdents64:       // EPERM not a valid errno.
+#if defined(__i386__)
+    case __NR_readdir:
+#endif
       return true;
     default:
       return false;
@@ -288,16 +338,37 @@ bool IsGetSimpleId(int sysno) {
   switch (sysno) {
     case __NR_capget:
     case __NR_getegid:
+#if defined(__i386__)
+    case __NR_getegid32:
+#endif
     case __NR_geteuid:
+#if defined(__i386__)
+    case __NR_geteuid32:
+#endif
     case __NR_getgid:
+#if defined(__i386__)
+    case __NR_getgid32:
+#endif
     case __NR_getgroups:
+#if defined(__i386__)
+    case __NR_getgroups32:
+#endif
     case __NR_getpid:
     case __NR_getppid:
     case __NR_getresgid:
+#if defined(__i386__)
+    case __NR_getresgid32:
+#endif
     case __NR_getresuid:
+#if defined(__i386__)
+    case __NR_getresuid32:
+#endif
     case __NR_getsid:
     case __NR_gettid:
     case __NR_getuid:
+#if defined(__i386__)
+    case __NR_getuid32:
+#endif
       return true;
     default:
       return false;
@@ -318,6 +389,17 @@ bool IsProcessPrivilegeChange(int sysno) {
     case __NR_setresuid:
     case __NR_setreuid:
     case __NR_setuid:
+#if defined(__i386__)
+    case __NR_setfsgid32:
+    case __NR_setfsuid32:
+    case __NR_setgid32:
+    case __NR_setgroups32:
+    case __NR_setregid32:
+    case __NR_setresgid32:
+    case __NR_setresuid32:
+    case __NR_setreuid32:
+    case __NR_setuid32:
+#endif
       return true;
     default:
       return false;
@@ -341,6 +423,11 @@ bool IsAllowedSignalHandling(int sysno) {
     case __NR_rt_sigaction:
     case __NR_rt_sigprocmask:
     case __NR_rt_sigreturn:
+#if defined(__i386__)
+    case __NR_sigaction:
+    case __NR_sigprocmask:
+    case __NR_sigreturn:
+#endif
       return true;
     case __NR_rt_sigpending:
     case __NR_rt_sigqueueinfo:
@@ -350,6 +437,13 @@ bool IsAllowedSignalHandling(int sysno) {
     case __NR_sigaltstack:
     case __NR_signalfd:
     case __NR_signalfd4:
+#if defined(__i386__)
+    case __NR_signal:
+    case __NR_sigpending:
+    case __NR_sigsuspend:
+    case __NR_sgetmask:  // Obsolete.
+    case __NR_ssetmask:
+#endif
     default:
       return false;
   }
@@ -362,7 +456,12 @@ bool IsOperationOnFd(int sysno) {
     case __NR_dup2:
     case __NR_dup3:
     case __NR_fcntl:  // TODO(jln): we may want to restrict arguments.
+#if defined(__i386__)
+    case __NR_fcntl64:
+#endif
+#if defined(__x86_64__)
     case __NR_shutdown:
+#endif
       return true;
     default:
       return false;
@@ -386,6 +485,9 @@ bool IsAllowedProcessStartOrDeath(int sysno) {
     case __NR_exit_group:
     case __NR_wait4:
     case __NR_waitid:
+#if defined(__i386__)
+    case __NR_waitpid:
+#endif
       return true;
     case __NR_setns:  // Privileged.
     case __NR_fork:
@@ -419,9 +521,13 @@ bool IsAllowedEpoll(int sysno) {
     case __NR_epoll_wait:
       return true;
     default:
+#if defined(__x86_64__)
     case __NR_epoll_ctl_old:
+#endif
     case __NR_epoll_pwait:
+#if defined(__x86_64__)
     case __NR_epoll_wait_old:
+#endif
       return false;
   }
 }
@@ -430,19 +536,35 @@ bool IsAllowedGetOrModifySocket(int sysno) {
   switch (sysno) {
     case __NR_pipe:
     case __NR_pipe2:
+#if defined(__x86_64__)
     case __NR_socketpair:  // We will want to inspect its argument.
+#endif
       return true;
     default:
+#if defined(__x86_64__)
     case __NR_accept:
     case __NR_accept4:
     case __NR_bind:
     case __NR_connect:
     case __NR_socket:
     case __NR_listen:
+#endif
+      return false;
+  }
+}
+
+#if defined(__i386__)
+bool IsSocketCall(int sysno) {

[Markus (顧孟勤), 2012/08/11 00:58:40]

We need a big comment that this is an incomplete and insecure solution.

[jln (very slow on Chromium), 2012/08/11 01:36:38]

Well there is really no security issue per se. It is "socketcall". Right now
it's filtered *out* by default in the baseline (and there really is no i386
policy yet).

Perhaps the comment will belong to the callsite when we make i386 policies ?

I have nonetheless added a comment above the function signature.

+  switch (sysno) {
+    case __NR_socketcall:
+      return true;
+    default:
       return false;
   }
 }
+#endif
 
+#if defined(__x86_64__)
 bool IsNetworkSocketInformation(int sysno) {
   switch (sysno) {
     case __NR_getpeername:
@@ -454,6 +576,7 @@ bool IsNetworkSocketInformation(int sysno) {
       return false;
   }
 }
+#endif
 
 bool IsAllowedAddressSpaceAccess(int sysno) {
   switch (sysno) {
@@ -467,12 +590,19 @@ bool IsAllowedAddressSpaceAccess(int sysno) {
       return true;
     case __NR_mincore:
     case __NR_mlockall:
+#if defined(__i386__)
+    case __NR_mmap2:  // Might need to be enabled but should be audited.

[Markus (顧孟勤), 2012/08/11 00:58:40]

mmap2() is the new one isn't it? mmap() is the old and broken API. It is
probably harmless, but we could disallow mmap(), if mmap2() exists.

[jln (very slow on Chromium), 2012/08/11 01:36:38]

Ohh yeah, good point, glibc converts the offset argument to mmap2 indeed.

Done.

+#endif
     case __NR_modify_ldt:
     case __NR_mremap:
     case __NR_msync:
     case __NR_munlockall:
     case __NR_readahead:
     case __NR_remap_file_pages:
+#if defined(__i386__)
+    case __NR_vm86:
+    case __NR_vm86old:
+#endif
     default:
       return false;
   }
@@ -481,16 +611,26 @@ bool IsAllowedAddressSpaceAccess(int sysno) {
 bool IsAllowedGeneralIo(int sysno) {
   switch (sysno) {
     case __NR_lseek:
+#if defined(__i386__)
+    case __NR__llseek:
+#endif
     case __NR_poll:
     case __NR_ppoll:
     case __NR_pselect6:
     case __NR_read:
     case __NR_readv:
+#if defined(__x86_64__)
     case __NR_recvfrom:  // Could specify source.
     case __NR_recvmsg:   // Could specify source.
+#endif
     case __NR_select:
+#if defined(__i386__)
+    case __NR__newselect:
+#endif
+#if defined(__x86_64__)
     case __NR_sendmsg:   // Could specify destination.
     case __NR_sendto:    // Could specify destination.
+#endif
     case __NR_write:
     case __NR_writev:
       return true;
@@ -501,6 +641,9 @@ bool IsAllowedGeneralIo(int sysno) {
     case __NR_pwritev:
     case __NR_recvmmsg:  // Could specify source.
     case __NR_sendfile:
+#if defined(__i386__)
+    case __NR_sendfile64:
+#endif
     case __NR_sendmmsg:  // Could specify destination.
     case __NR_splice:
     case __NR_tee:
@@ -515,7 +658,9 @@ bool IsAllowedPrctl(int sysno) {
     case __NR_prctl:
       return true;
     default:
+#if defined(__x86_64__)
     case __NR_arch_prctl:
+#endif
       return false;
   }
 }
@@ -527,6 +672,9 @@ bool IsAllowedBasicScheduler(int sysno) {
     case __NR_nanosleep:
       return true;
     case __NR_getpriority:
+#if defined(__i386__)
+    case __NR_nice:
+#endif
     case __NR_setpriority:
     default:
       return false;
@@ -535,6 +683,9 @@ bool IsAllowedBasicScheduler(int sysno) {
 
 bool IsAdminOperation(int sysno) {
   switch (sysno) {
+#if defined(__i386__)
+    case __NR_bdflush:
+#endif
     case __NR_kexec_load:
     case __NR_reboot:
     case __NR_setdomainname:
@@ -577,6 +728,9 @@ bool IsFsControl(int sysno) {
     case __NR_quotactl:
     case __NR_swapoff:
     case __NR_swapon:
+#if defined(__i386__)
+    case __NR_umount:
+#endif
     case __NR_umount2:
       return true;
     default:
@@ -616,6 +770,10 @@ bool IsGlobalProcessEnvironment(int sysno) {
   switch (sysno) {
     case __NR_acct:         // Privileged.
     case __NR_getrlimit:
+#if defined(__i386__)
+    case __NR_ugetrlimit:
+    case __NR_ulimit:
+#endif
     case __NR_getrusage:
     case __NR_personality:  // Can change its personality as well.
     case __NR_prlimit64:    // Like setrlimit / getrlimit.
@@ -645,6 +803,10 @@ bool IsGlobalSystemStatus(int sysno) {
     case __NR_sysfs:
     case __NR_sysinfo:
     case __NR_uname:
+#if defined(__i386__)
+    case __NR_olduname:
+    case __NR_oldolduname:
+#endif
       return true;
     default:
       return false;
@@ -686,6 +848,7 @@ bool IsKeyManagement(int sysno) {
   }
 }
 
+#if defined(__x86_64__)
 bool IsSystemVSemaphores(int sysno) {
   switch (sysno) {
     case __NR_semctl:
@@ -697,7 +860,9 @@ bool IsSystemVSemaphores(int sysno) {
       return false;
   }
 }
+#endif
 
+#if defined(__x86_64__)
 // These give a lot of ambient authority and bypass the setuid sandbox.
 bool IsAllowedSystemVSharedMemory(int sysno) {
   switch (sysno) {
@@ -710,7 +875,9 @@ bool IsAllowedSystemVSharedMemory(int sysno) {
       return false;
   }
 }
+#endif
 
+#if defined(__x86_64__)
 bool IsSystemVMessageQueue(int sysno) {
   switch (sysno) {
     case __NR_msgctl:
@@ -722,6 +889,18 @@ bool IsSystemVMessageQueue(int sysno) {
       return false;
   }
 }
+#endif
+
+#if defined(__i386__)
+bool IsSystemVIpc(int sysno) {

[Markus (顧孟勤), 2012/08/11 00:58:40]

Again, we probably want a comment that this is incomplete and not particularly
safe.

[jln (very slow on Chromium), 2012/08/11 01:36:38]

I moved the comment below above the function signature.

+  switch (sysno) {
+    case __NR_ipc:  // Big system V multiplexing system call.
+      return true;
+    default:
+      return false;
+  }
+}
+#endif
 
 bool IsAdvancedScheduler(int sysno) {
   switch (sysno) {
@@ -822,10 +1001,28 @@ bool IsMisc(int sysno) {
     case __NR_vhangup:
     // The system calls below are not implemented.
     case __NR_afs_syscall:
+#if defined(__i386__)
+    case __NR_break:
+#endif
     case __NR_getpmsg:
+#if defined(__i386__)
+    case __NR_gtty:
+    case __NR_idle:
+    case __NR_lock:
+    case __NR_mpx:
+    case __NR_prof:
+    case __NR_profil:
+#endif
     case __NR_putpmsg:
+#if defined(__x86_64__)
     case __NR_security:
+#endif
+#if defined(__i386__)
+    case __NR_stty:
+#endif
+#if defined(__x86_64__)
     case __NR_tuxcall:
+#endif
     case __NR_vserver:
       return true;
     default:
@@ -863,7 +1060,9 @@ bool IsBaselinePolicyWatched_x86_64(int sysno) {
   if (IsAdminOperation(sysno) ||
       IsAdvancedScheduler(sysno) ||
       IsAdvancedTimer(sysno) ||
+#if defined(__x86_64__)
       IsAllowedSystemVSharedMemory(sysno) ||
+#endif
       IsAsyncIo(sysno) ||
       IsDebug(sysno) ||
       IsEventFd(sysno) ||
@@ -878,12 +1077,22 @@ bool IsBaselinePolicyWatched_x86_64(int sysno) {
       IsKeyManagement(sysno) ||
       IsMessageQueue(sysno) ||
       IsMisc(sysno) ||
+#if defined(__x86_64__)
       IsNetworkSocketInformation(sysno) ||
+#endif
       IsNuma(sysno) ||
       IsProcessGroupOrSession(sysno) ||
       IsProcessPrivilegeChange(sysno) ||
+#if defined(__i386__)
+      IsSocketCall(sysno) ||  // We'll need to handle this properly to build
+                              // a x86_32 policy.
+#endif
+#if defined(__x86_64__)
       IsSystemVMessageQueue(sysno) ||
       IsSystemVSemaphores(sysno) ||
+#elif defined(__i386__)
+      IsSystemVIpc(sysno) ||
+#endif
       IsTimer(sysno)) {
     return true;
   } else {
@@ -919,8 +1128,10 @@ playground2::Sandbox::ErrorCode GpuProcessPolicy_x86_64(int sysno) {
   switch(sysno) {
     case __NR_ioctl:
       return playground2::Sandbox::SB_ALLOWED;
+#if defined(__x86_64__)
     case __NR_socket:
       return EACCES;  // Nvidia binary driver.
+#endif
     case __NR_open:
       // Accelerated video decode is enabled by default only on Chrome OS.
       if (IsAcceleratedVideoDecodeEnabled()) {
@@ -954,19 +1165,23 @@ playground2::Sandbox::ErrorCode FlashProcessPolicy_x86_64(int sysno) {
       return playground2::Sandbox::SB_ALLOWED;
     case __NR_ioctl:
       return ENOTTY;  // Flash Access.
+#if defined(__x86_64__)
     case __NR_socket:
       return EACCES;
+#endif
     default:
+#if defined(__x86_64__)
       // These are under investigation, and hopefully not here for the long
       // term.
       if (IsAllowedSystemVSharedMemory(sysno))
         return playground2::Sandbox::SB_ALLOWED;
+#endif
 
       // Default on the baseline policy.
       return  BaselinePolicy_x86_64(sysno);
   }
 }
-#endif
+#endif  // defined(__x86_64__) || defined(__i386__)
 
 playground2::Sandbox::ErrorCode BlacklistPtracePolicy(int sysno) {
   if (sysno < static_cast<int>(MIN_SYSCALL) ||