914716 - __NR_recv, __NR_llseek, __NR_fcntl64, etc. undeclared on x86_64 when compiling with seccomp-bpf sandboxing

Status: RESOLVED FIXED

(Core :: Security, defect)

Description

[Dana Keeler (she/her) (use needinfo) [:keeler]]

The full list is:
__NR_recv
__NR__llseek
__NR_sigreturn
__NR_fcntl64
__NR_mmap2
__NR_fstat64
__NR_stat64
__NR_sigprocmask

Bug 909658 handled a similar situation, but I think we should have a more general solution:
- have a list of common syscalls defined on every platform
- have a list of syscalls particular to each platform
- make the full whitelist out of the union of the common list and the appropriate platform list (using #ifdef (__i386__) or whatever).

Comment 1

[Dana Keeler (she/her) (use needinfo) [:keeler]]

looks like the list has been updated to include getuid32, which also isn't defined on x86_64.

Comment 2

[Dana Keeler (she/her) (use needinfo) [:keeler]]

Attachment: patch wip

Guillaume - what do you think about this kind of approach?

Comment 3

[Guillaume Destuynder [:kang] [this account is no longer active]]

Comment on attachment 803168 [details] [diff] [review]
patch wip

Review of attachment 803168 [details] [diff] [review]:
-----------------------------------------------------------------

There is currently one issue with this approach, it moves less-often-called syscalls to the top of the list.
Maybe another possibility is to have something like:

SECCOMP_ARCH_HIGH //most often called, often many times a second
recv,ipc,read,...
SECCOMP_ARCH_MED // rarely called
getuid,...
SECCOMP_ARCH_LOW //nearly never called
exit,...
SECCOMP_ARCH_REMOVE_US //has to be removed when APIs are fixed
open,...

where ARCH is x86_32/x86_64/arm/etc. in fact, i think MED is not necessary, HIGH and LOW should be enough.

::: dom/ipc/ContentProcess.cpp
@@ +32,5 @@
>      mXREEmbed.Start();
>      mContent.InitXPCOM();
>      
> +#ifdef MOZ_CONTENT_SANDBOX
> +    mozilla::SetCurrentProcessSandbox();

this unfortunately doesn't work well right now on b2g, AFAIK - not everything is initialized at this stage.

Comment 4

[Dana Keeler (she/her) (use needinfo) [:keeler]]

Attachment: patch

Thanks for the feedback!
I decided to narrow the scope of this bug a bit to just "make seccomp-bpf sandboxing compile on x86_64". Also, I split the architecture-specific calls into HIGH and LOW, but I'm not sure I put the right calls in the right groups.

Comment 5

[Dana Keeler (she/her) (use needinfo) [:keeler]]

Comment on attachment 806189 [details] [diff] [review]
patch

kang and I spoke on IRC - I need to re-order some of these syscalls. Clearing r? for now.

Comment 6

[Dana Keeler (she/her) (use needinfo) [:keeler]]

Attachment: patch v2

Ok - I re-ordered some of the syscalls according to https://wiki.mozilla.org/B2G/Architecture/System_Security/Seccomp
Let me know what you think.

Comment 7

[Guillaume Destuynder [:kang] [this account is no longer active]]

Comment on attachment 810059 [details] [diff] [review]
patch v2

Review of attachment 810059 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me

::: security/sandbox/seccomp_filter.h
@@ +110,5 @@
>    /* These are calls we're ok to allow */ \
> +  SECCOMP_WHITELIST_ADD_ARM_HIGH \
> +  SECCOMP_WHITELIST_ADD_i386_HIGH \
> +  SECCOMP_WHITELIST_ADD_x86_64_HIGH \
> +  ALLOW_SYSCALL(gettimeofday), \

gettimeofday could almost be above the HIGH list so that it's above mmap2 - but given the short list I suppose it wont make a big difference for now.

Comment 8

[Dana Keeler (she/her) (use needinfo) [:keeler]]

Thanks!
https://hg.mozilla.org/integration/mozilla-inbound/rev/9062838e95de

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/9062838e95de