content: bpf: exclude the syscalls if arm64

content/common/sandbox_linux/bpf_cros_arm_gpu_policy_linux.cc

Index: content/common/sandbox_linux/bpf_cros_arm_gpu_policy_linux.cc
diff --git a/content/common/sandbox_linux/bpf_cros_arm_gpu_policy_linux.cc b/content/common/sandbox_linux/bpf_cros_arm_gpu_policy_linux.cc
index de0a89f5ed9d16ac485cc356d215fe9ca61a3286..c679d236ff93ae2e7bcb7023caa8d03b7f6b12b7 100644
--- a/content/common/sandbox_linux/bpf_cros_arm_gpu_policy_linux.cc
+++ b/content/common/sandbox_linux/bpf_cros_arm_gpu_policy_linux.cc
@@ -48,7 +48,7 @@ inline bool IsChromeOS() {
 }
 
 inline bool IsArchitectureArm() {
-#if defined(__arm__)
+#if defined(__arm__) || defined(__aarch64__)
   return true;
 #else
   return false;
@@ -108,8 +108,11 @@ class CrosArmGpuBrokerProcessPolicy : public CrosArmGpuProcessPolicy {
 // openat allowed.
 ResultExpr CrosArmGpuBrokerProcessPolicy::EvaluateSyscall(int sysno) const {
   switch (sysno) {
+#if !defined(aarch64)

[jln (very slow on Chromium), 2014/12/08 19:09:24]

You mean __aarch64__, no?

[rickyz (no longer on Chrome), 2014/12/08 19:22:07]

Would #if defined(__NR_access) and similar be more a more self-documenting check
here and elsewhere?

[cometzero, 2014/12/09 17:38:42]

Hmm. Looks good. To do that, however, we should change all of the syscalls to
support such self-checks.

[cometzero, 2014/12/09 17:38:42]

Oops. You're right. I'll fix it.

[rickyz (no longer on Chrome), 2014/12/09 20:34:14]

Sorry, I don't understand - does #if defined(__NR_access) not work currently? I
thought the macros wouldn't be defined on architectures where the syscall isn't
supported.

[jln (very slow on Chromium), 2014/12/09 20:39:01]

Yes, I think that's correct, it should work (we have linux_syscalls.h to make
sure that everything is defined, regardless of the system's header files).

However, I do not want to mix the two models, of architecture / syscall
existence #ifdefs.

We did make a conscious choice to use the latter, rather than the former. I do
still think that overall this works better, but I could be convinced otherwise.
If we do decide we want to switch to a #ifdef(syscall) model, we should write
the pros / cons in a bug and then do so consistently and it should be
independent of this CL.

[rickyz (no longer on Chrome), 2014/12/09 20:43:33]

Ah, I see, I had not fully grasped what you were saying in your other comment
before. Never mind about this for now then.

     case __NR_access:
     case __NR_open:
+#endif // !defined(aarch64)

[rickyz (no longer on Chrome), 2014/12/08 19:22:07]

nit: 2 spaces before the // here and elsewhere

[cometzero, 2014/12/09 17:38:42]

Thanks. I'll fix it.

+    case __NR_faccessat:
     case __NR_openat:
       return Allow();
     default:
@@ -125,13 +128,13 @@ CrosArmGpuProcessPolicy::CrosArmGpuProcessPolicy(bool allow_shmat)
 CrosArmGpuProcessPolicy::~CrosArmGpuProcessPolicy() {}
 
 ResultExpr CrosArmGpuProcessPolicy::EvaluateSyscall(int sysno) const {
-#if defined(__arm__)
+#if defined(__arm__) || defined(__aarch64__)

[rickyz (no longer on Chrome), 2014/12/08 19:22:07]

Are any of these #ifs in this file actually needed? The policy is called
CrosArmGpuProcessPolicy, so I suspect these ifs might be left over from before
we had a separate class for the ARM policy.

[jln (very slow on Chromium), 2014/12/08 19:38:10]

At some point we could consider doing that, but for consistency, let's keep the
architecture version for now in this small patch.

It's nice to be able to immediately gauge the security depending on the
architecture, so I'm not certain we'll want to switch, but I could be convinced.

[cometzero, 2014/12/09 17:38:42]

I think arm and arm64 are using similar GPU like mali and have similar policy
for them except syscall numbers.

BTW, I prepared below three patches on second patch. However, they were merged
into only one patch during posting second patch.

ea26ef9 content: bpf: add __NR_faccessat syscall
e298c77 content: bpf: support aarch64 syscalls likewise arm
b1c8dac content: bpf: exclude the syscalls if arm64

   if (allow_shmat_ && sysno == __NR_shmat)
     return Allow();
-#endif  // defined(__arm__)
+#endif  // defined(__arm__) || defined(__aarch64__)
 
   switch (sysno) {
-#if defined(__arm__)
+#if defined(__arm__) || defined(__aarch64__)
     // ARM GPU sandbox is started earlier so we need to allow networking
     // in the sandbox.
     case __NR_connect:
@@ -146,7 +149,7 @@ ResultExpr CrosArmGpuProcessPolicy::EvaluateSyscall(int sysno) const {
       const Arg<int> domain(0);
       return If(domain == AF_UNIX, Allow()).Else(Error(EPERM));
     }
-#endif  // defined(__arm__)
+#endif  // defined(__arm__) || defined(__aarch64__)
     default:
       // Default to the generic GPU policy.
       return GpuProcessPolicy::EvaluateSyscall(sysno);