sandbox: Add Arm64 support for seccomp-BPF

sandbox: Add Arm64 support for seccomp-BPF

Adds support for Arm64 to 'sandbox/'. This can be used
by future CLs to provide Arm64 policy for Chrome.

BUG=355125
TEST=sandbox_linux_unittests

Committed: https://crrev.com/e6f6b730e646a11e6f6a3c1c5f54e052aa07584a
Cr-Commit-Position: refs/heads/master@{#291631}

[leecam, 2014-08-21 11:34:48]

PTAL. Still need to fix __NR_setpriority and __NR_sigaltstack as I'm allowing
them in the baseline policy for Arm64. Will correct this to fit in with jln
patch for this issue.

[jln (very slow on Chromium), 2014-08-21 19:04:54]

Great work Lee!

Please, make it clear in the commit message that this adds support for ARM64 in
sandbox/ only (as opposed to a working policy on Chrome).

A few nits, but otherwise this looks good.

https://chromiumcodereview.appspot.com/487143003/diff/40001/sandbox/linux/sec...
File sandbox/linux/seccomp-bpf-helpers/baseline_policy_unittest.cc (right):

https://chromiumcodereview.appspot.com/487143003/diff/40001/sandbox/linux/sec...
sandbox/linux/seccomp-bpf-helpers/baseline_policy_unittest.cc:239: #if
!defined(__aarch64__)
I'm not sure why this list is not sorted (almost certainly my fault).

Let's sort it please, and extract the non aarch64 ones and put them under (after
a vertical whitespace) in a new "section".

https://chromiumcodereview.appspot.com/487143003/diff/40001/sandbox/linux/sec...
File sandbox/linux/seccomp-bpf-helpers/syscall_sets.cc (right):

https://chromiumcodereview.appspot.com/487143003/diff/40001/sandbox/linux/sec...
sandbox/linux/seccomp-bpf-helpers/syscall_sets.cc:79: #if !defined(__aarch64__)
Alphabetical order is important.

But I also understand that having 20 "#if !defined()" would look awful.

How about making two different "sections", separated with a blank line, but
every section would be sorted properly?

https://chromiumcodereview.appspot.com/487143003/diff/40001/sandbox/linux/sec...
sandbox/linux/seccomp-bpf-helpers/syscall_sets.cc:593: bool
SyscallSets::IsSeccomp(int sysno) {
I don't think we need this for now, right?

If we "complete" the system call sets, we should also add the 4-5 system calls
that have been added since I last maintained these files and the addition of
seccomp. Let's do that in another CL instead?

https://chromiumcodereview.appspot.com/487143003/diff/40001/sandbox/linux/sec...
sandbox/linux/seccomp-bpf-helpers/syscall_sets.cc:608: case __NR_pause:
I didn't realize that pause() was deprecated. Is it wrapped around
 sigsuspend() in glibc?

https://chromiumcodereview.appspot.com/487143003/diff/40001/sandbox/linux/sec...
sandbox/linux/seccomp-bpf-helpers/syscall_sets.cc:648: case __NR_finit_module:
This is not AARCH64 specific. Maybe add any missing syscalls in another CL?

https://chromiumcodereview.appspot.com/487143003/diff/40001/sandbox/linux/sec...
File sandbox/linux/seccomp-bpf/sandbox_bpf_unittest.cc (right):

https://chromiumcodereview.appspot.com/487143003/diff/40001/sandbox/linux/sec...
sandbox/linux/seccomp-bpf/sandbox_bpf_unittest.cc:1227: static const int
kNumTestCases = 30;
Maybe let's change it depending on architecture?

I wouldn't want to reduce test coverage on existing archs.

https://chromiumcodereview.appspot.com/487143003/diff/40001/sandbox/linux/sec...
sandbox/linux/seccomp-bpf/sandbox_bpf_unittest.cc:2005: printf("This test is
currently disabled on ARM64..");
Could you add a link to the open bug about this? (and above as well).

https://chromiumcodereview.appspot.com/487143003/diff/40001/sandbox/linux/sec...
File sandbox/linux/seccomp-bpf/syscall.cc (right):

https://chromiumcodereview.appspot.com/487143003/diff/40001/sandbox/linux/sec...
sandbox/linux/seccomp-bpf/syscall.cc:228: #elif defined(__aarch64__)
I did not review this yet.

https://chromiumcodereview.appspot.com/487143003/diff/40001/sandbox/linux/sec...
File sandbox/linux/seccomp-bpf/syscall_unittest.cc (right):

https://chromiumcodereview.appspot.com/487143003/diff/40001/sandbox/linux/sec...
sandbox/linux/seccomp-bpf/syscall_unittest.cc:160: fd =
Syscall::Call(__NR_openat, AT_FDCWD, "/dev/null", O_RDWR, 0L));
Could we use openat everywhere instead? It should work on every arch.

https://chromiumcodereview.appspot.com/487143003/diff/40001/sandbox/linux/sec...
sandbox/linux/seccomp-bpf/syscall_unittest.cc:195: #if defined(__aarch64__)
Same remark.

https://chromiumcodereview.appspot.com/487143003/diff/40001/sandbox/linux/ser...
File sandbox/linux/services/arm64_linux_syscalls.h (right):

https://chromiumcodereview.appspot.com/487143003/diff/40001/sandbox/linux/ser...
sandbox/linux/services/arm64_linux_syscalls.h:1040: #endif
Let's not have holes here. See Robert's CL at
https://chromiumcodereview.appspot.com/494743003/

(you may want to simply rebase on top of it).

[leecam, 2014-08-22 10:44:18]

https://codereview.chromium.org/487143003/diff/40001/sandbox/linux/seccomp-bp...
File sandbox/linux/seccomp-bpf-helpers/baseline_policy_unittest.cc (right):

https://codereview.chromium.org/487143003/diff/40001/sandbox/linux/seccomp-bp...
sandbox/linux/seccomp-bpf-helpers/baseline_policy_unittest.cc:239: #if
!defined(__aarch64__)
On 2014/08/21 19:04:53, jln wrote:
> I'm not sure why this list is not sorted (almost certainly my fault).
> 
> Let's sort it please, and extract the non aarch64 ones and put them under
(after
> a vertical whitespace) in a new "section".

Done.

https://codereview.chromium.org/487143003/diff/40001/sandbox/linux/seccomp-bp...
File sandbox/linux/seccomp-bpf-helpers/syscall_sets.cc (right):

https://codereview.chromium.org/487143003/diff/40001/sandbox/linux/seccomp-bp...
sandbox/linux/seccomp-bpf-helpers/syscall_sets.cc:79: #if !defined(__aarch64__)
On 2014/08/21 19:04:53, jln wrote:
> Alphabetical order is important.
> 
> But I also understand that having 20 "#if !defined()" would look awful.
> 
> How about making two different "sections", separated with a blank line, but
> every section would be sorted properly?

Done

https://codereview.chromium.org/487143003/diff/40001/sandbox/linux/seccomp-bp...
sandbox/linux/seccomp-bpf-helpers/syscall_sets.cc:593: bool
SyscallSets::IsSeccomp(int sysno) {
On 2014/08/21 19:04:53, jln wrote:
> I don't think we need this for now, right?
> 
> If we "complete" the system call sets, we should also add the 4-5 system calls
> that have been added since I last maintained these files and the addition of
> seccomp. Let's do that in another CL instead?
Moved to another CL

https://codereview.chromium.org/487143003/diff/40001/sandbox/linux/seccomp-bp...
sandbox/linux/seccomp-bpf-helpers/syscall_sets.cc:608: case __NR_pause:
On 2014/08/21 19:04:53, jln wrote:
> I didn't realize that pause() was deprecated. Is it wrapped around
>  sigsuspend() in glibc?

Yeah glibc just calls sigsuspend().

https://codereview.chromium.org/487143003/diff/40001/sandbox/linux/seccomp-bp...
sandbox/linux/seccomp-bpf-helpers/syscall_sets.cc:648: case __NR_finit_module:
On 2014/08/21 19:04:53, jln wrote:
> This is not AARCH64 specific. Maybe add any missing syscalls in another CL?
Moved to another CL

https://codereview.chromium.org/487143003/diff/40001/sandbox/linux/seccomp-bp...
File sandbox/linux/seccomp-bpf/sandbox_bpf_unittest.cc (right):

https://codereview.chromium.org/487143003/diff/40001/sandbox/linux/seccomp-bp...
sandbox/linux/seccomp-bpf/sandbox_bpf_unittest.cc:1227: static const int
kNumTestCases = 30;
On 2014/08/21 19:04:53, jln wrote:
> Maybe let's change it depending on architecture?
> 
> I wouldn't want to reduce test coverage on existing archs.

Done.

https://codereview.chromium.org/487143003/diff/40001/sandbox/linux/seccomp-bp...
sandbox/linux/seccomp-bpf/sandbox_bpf_unittest.cc:2005: printf("This test is
currently disabled on ARM64..");
On 2014/08/21 19:04:53, jln wrote:
> Could you add a link to the open bug about this? (and above as well).

Done.

https://codereview.chromium.org/487143003/diff/40001/sandbox/linux/seccomp-bp...
File sandbox/linux/seccomp-bpf/syscall_unittest.cc (right):

https://codereview.chromium.org/487143003/diff/40001/sandbox/linux/seccomp-bp...
sandbox/linux/seccomp-bpf/syscall_unittest.cc:160: fd =
Syscall::Call(__NR_openat, AT_FDCWD, "/dev/null", O_RDWR, 0L));
On 2014/08/21 19:04:53, jln wrote:
> Could we use openat everywhere instead? It should work on every arch.

Done.

https://codereview.chromium.org/487143003/diff/40001/sandbox/linux/seccomp-bp...
sandbox/linux/seccomp-bpf/syscall_unittest.cc:195: #if defined(__aarch64__)
On 2014/08/21 19:04:53, jln wrote:
> Same remark.

Done.

https://codereview.chromium.org/487143003/diff/40001/sandbox/linux/services/a...
File sandbox/linux/services/arm64_linux_syscalls.h (right):

https://codereview.chromium.org/487143003/diff/40001/sandbox/linux/services/a...
sandbox/linux/services/arm64_linux_syscalls.h:1040: #endif
On 2014/08/21 19:04:54, jln wrote:
> Let's not have holes here. See Robert's CL at
> https://chromiumcodereview.appspot.com/494743003/
> 
> (you may want to simply rebase on top of it).

Done.

[jln (very slow on Chromium), 2014-08-22 20:11:30]

lgtm with small nits.

I only glanced over the asm trampoline in syscall.cc. I think it's fine for now,
we can iterate later.

https://codereview.chromium.org/487143003/diff/60001/sandbox/linux/seccomp-bp...
File sandbox/linux/seccomp-bpf-helpers/baseline_policy.cc (right):

https://codereview.chromium.org/487143003/diff/60001/sandbox/linux/seccomp-bp...
sandbox/linux/seccomp-bpf-helpers/baseline_policy.cc:131: return Allow();
Nit: can y ou move this above with the other allow, before the section with
"Restrict*" ?

https://codereview.chromium.org/487143003/diff/60001/sandbox/linux/seccomp-bp...
File sandbox/linux/seccomp-bpf/sandbox_bpf_unittest.cc (right):

https://codereview.chromium.org/487143003/diff/60001/sandbox/linux/seccomp-bp...
sandbox/linux/seccomp-bpf/sandbox_bpf_unittest.cc:1228: static const int
kNumTestCases = 30;
Did you figure out what was happening btw?

CodeGen::PrintProgram() is a useful debugging tool, but these days, there are
also a bunch of other BPF decompilers / debuggers.

https://codereview.chromium.org/487143003/diff/60001/sandbox/linux/seccomp-bp...
sandbox/linux/seccomp-bpf/sandbox_bpf_unittest.cc:2003: // The following tests
are disabled due to a kernel bug.
You mean "The current test"?

https://codereview.chromium.org/487143003/diff/60001/sandbox/linux/seccomp-bp...
sandbox/linux/seccomp-bpf/sandbox_bpf_unittest.cc:2005: #if defined(__arm__)
You could probably just || (__aarch64__) here, with a common message.

[leecam, 2014-08-24 22:11:58]

https://codereview.chromium.org/487143003/diff/60001/sandbox/linux/seccomp-bp...
File sandbox/linux/seccomp-bpf-helpers/baseline_policy.cc (right):

https://codereview.chromium.org/487143003/diff/60001/sandbox/linux/seccomp-bp...
sandbox/linux/seccomp-bpf-helpers/baseline_policy.cc:131: return Allow();
On 2014/08/22 20:11:30, jln wrote:
> Nit: can y ou move this above with the other allow, before the section with
> "Restrict*" ?

Done.

https://codereview.chromium.org/487143003/diff/60001/sandbox/linux/seccomp-bp...
File sandbox/linux/seccomp-bpf/sandbox_bpf_unittest.cc (right):

https://codereview.chromium.org/487143003/diff/60001/sandbox/linux/seccomp-bp...
sandbox/linux/seccomp-bpf/sandbox_bpf_unittest.cc:1228: static const int
kNumTestCases = 30;
On 2014/08/22 20:11:30, jln wrote:
> Did you figure out what was happening btw?
> 
> CodeGen::PrintProgram() is a useful debugging tool, but these days, there are
> also a bunch of other BPF decompilers / debuggers.
I'm still looking into this one but I don't think there is anything
fundamentally bad happening

https://codereview.chromium.org/487143003/diff/60001/sandbox/linux/seccomp-bp...
sandbox/linux/seccomp-bpf/sandbox_bpf_unittest.cc:2003: // The following tests
are disabled due to a kernel bug.
On 2014/08/22 20:11:30, jln wrote:
> You mean "The current test"?

Yep ;-)

https://codereview.chromium.org/487143003/diff/60001/sandbox/linux/seccomp-bp...
sandbox/linux/seccomp-bpf/sandbox_bpf_unittest.cc:2005: #if defined(__arm__)
On 2014/08/22 20:11:30, jln wrote:
> You could probably just || (__aarch64__) here, with a common message.

Done.

[leecam, 2014-08-24 22:13:21]

The CQ bit was checked by leecam@chromium.org

[commit-bot: I haz the power, 2014-08-24 22:14:16]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/leecam@chromium.org/487143003/80001

[commit-bot: I haz the power, 2014-08-24 23:06:53]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  mac_chromium_rel_swarming on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[commit-bot: I haz the power, 2014-08-24 23:38:29]

Committed patchset #5 (80001) as 2f4ad7efe924652f9cafa7de0a6eb2e2cdaf075f

[commit-bot: I haz the power, 2014-09-10 02:32:48]

Patchset 5 (id:??) landed as
https://crrev.com/e6f6b730e646a11e6f6a3c1c5f54e052aa07584a
Cr-Commit-Position: refs/heads/master@{#291631}