Closed
Bug 914716
Opened 11 years ago
Closed 11 years ago
__NR_recv, __NR_llseek, __NR_fcntl64, etc. undeclared on x86_64 when compiling with seccomp-bpf sandboxing
Categories
(Core :: Security, defect)
Tracking
()
RESOLVED
FIXED
mozilla27
People
(Reporter: keeler, Assigned: keeler)
Details
Attachments
(1 file, 2 obsolete files)
7.25 KB,
patch
|
kang
:
review+
|
Details | Diff | Splinter Review |
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).
Assignee | ||
Comment 1•11 years ago
|
||
looks like the list has been updated to include getuid32, which also isn't defined on x86_64.
Assignee | ||
Comment 2•11 years ago
|
||
Guillaume - what do you think about this kind of approach?
Attachment #803168 -
Flags: feedback?(gdestuynder)
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.
Attachment #803168 -
Flags: feedback?(gdestuynder)
Assignee | ||
Comment 4•11 years ago
|
||
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.
Assignee: nobody → dkeeler
Attachment #803168 -
Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #806189 -
Flags: review?(gdestuynder)
Assignee | ||
Comment 5•11 years ago
|
||
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.
Attachment #806189 -
Flags: review?(gdestuynder)
Assignee | ||
Comment 6•11 years ago
|
||
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.
Attachment #806189 -
Attachment is obsolete: true
Attachment #810059 -
Flags: review?(gdestuynder)
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.
Attachment #810059 -
Flags: review?(gdestuynder) → review+
Assignee | ||
Comment 8•11 years ago
|
||
Thanks! https://hg.mozilla.org/integration/mozilla-inbound/rev/9062838e95de
Comment 9•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/9062838e95de
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
You need to log in
before you can comment on or make changes to this bug.
Description
•