firefox seccomp-bpf sandbox is broken on musl libc

RESOLVED FIXED in Firefox 56

Status

()

defect
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: nsz.foo, Assigned: jld)

Tracking

54 Branch
mozilla56
Points:
---

Firefox Tracking Flags

(firefox56 fixed)

Details

(Whiteboard: sb+)

Attachments

(3 attachments)

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:54.0) Gecko/20100101 Firefox/54.0
Build ID: 20170626211010

Steps to reproduce:

start firefox 54.0 on alpine linux


Actual results:

instead of correct page load it shows "Gah. Your tab just crashed."

background process with seccomp-bpf filter crashed because pthread_create of musl uses different clone flags than glibc so the syscall failed and this lead to a null pointer dereference.

thread creation fails at ProcessHangMonitor ctor
https://dxr.mozilla.org/mozilla-central/source/dom/ipc/ProcessHangMonitor.cpp#1108
null deref is at CreateHangMonitorChild
https://dxr.mozilla.org/mozilla-central/source/dom/ipc/ProcessHangMonitor.cpp#1228
because mThread is 0.

the sandbox policy only allows the glibc clone flags and it fails to whitelist certain syscalls used by musl that are reasonable to allow, details are in the alpine bug report:
https://bugs.alpinelinux.org/issues/7454



Expected results:

allow thread creation, not crash.
Component: Untriaged → DOM
Product: Firefox → Core
Bill might be able to offer some thoughts ...
Flags: needinfo?(wmccloskey)
Flags: needinfo?(wmccloskey) → needinfo?(jld)
Assignee: nobody → jld
Component: DOM → Security: Process Sandboxing
Flags: needinfo?(jld)
To summarize what I'm seeing in the downstream bug report, and what to do about it:

1. musl libc is passing CLONE_DETACHED, of which the man page says: “This flag is still defined, but has no effect.” (as of 2.6.2, which predates seccomp-bpf); the kernel doesn't seem to have done anything with the flag other than #define it since the beginning of the Git history.  We can just mask it off in the flag comparison.

2. musl uses tkill to target threads in the current process; the workaround we did for Bionic in bug 1093893 can have its conditional expanded.

3. getdents vs. getdents64; normally amd64 doesn't have separate "64" versions of file-related syscalls, but apparently this is an exception.  Trivial to fix.

4. ALSA is trying to fork/exec to… run a shell to expand a glob?  There seems to be a workaround to make ALSA not do that, but if there weren't:

  a. ALSA support is Tier 3 (in the sense of https://developer.mozilla.org/en-US/docs/Supported_build_configurations), and plugins other than hw and dmix may already be broken by content sandboxing.  Up to a point we'd take patches that allow more things under #ifdef MOZ_ALSA, but:

  b. fork/exec wouldn't work even if we allowed the syscalls, because the child process would inherit the seccomp-bpf policy, and *not* the SIGSYS handler.  So the first time the dynamic linker calls open() or stat(), the process is immediately killed.  (Also, once we get content processes chrooted, /bin/sh won't exist because / will be an empty deleted directory.)
Whiteboard: sb+
Comment on attachment 8887219 [details]
Bug 1376653 - Loosen restrictions on clone flags for musl.

https://reviewboard.mozilla.org/r/158008/#review163708
Attachment #8887219 - Flags: review?(gpascutto) → review+
Comment on attachment 8887220 [details]
Bug 1376653 - Unconditionalize the tkill() polyfill.

https://reviewboard.mozilla.org/r/158010/#review163712
Attachment #8887220 - Flags: review?(gpascutto) → review+
Comment on attachment 8887221 [details]
Bug 1376653 - Fix handling of architecture differences for getdents.

https://reviewboard.mozilla.org/r/158012/#review163714
Attachment #8887221 - Flags: review?(gpascutto) → review+
Pushed by jedavis@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a8f06d32af31
Loosen restrictions on clone flags for musl. r=gcp
https://hg.mozilla.org/integration/autoland/rev/9b5bb669d128
Unconditionalize the tkill() polyfill. r=gcp
https://hg.mozilla.org/integration/autoland/rev/f68747fe8a15
Fix handling of architecture differences for getdents. r=gcp
https://hg.mozilla.org/mozilla-central/rev/a8f06d32af31
https://hg.mozilla.org/mozilla-central/rev/9b5bb669d128
https://hg.mozilla.org/mozilla-central/rev/f68747fe8a15
Status: UNCONFIRMED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in before you can comment on or make changes to this bug.