Closed
Bug 1286324
Opened 8 years ago
Closed 8 years ago
Seccomp sandbox violation: sys_clone called in content process of Firefox desktop
Categories
(Core :: Security: Process Sandboxing, defect)
Tracking
()
RESOLVED
FIXED
mozilla50
Tracking | Status | |
---|---|---|
firefox50 | --- | fixed |
People
(Reporter: tedd, Assigned: jld)
References
Details
(Whiteboard: sblc1)
Crash Data
Attachments
(1 file)
(deleted),
patch
|
tedd
:
review+
|
Details | Diff | Splinter Review |
Crash reports show that sys_sysfs is called in the content process:
https://crash-stats.mozilla.com/search/?product=Firefox&reason=~SIGSYS&address=0x78&_sort=-date&_facets=cpu_arch&_facets=address&_columns=date&_columns=signature&_columns=product&_columns=version&_columns=build_id&_columns=platform#crash-reports
Reporter | ||
Comment 1•8 years ago
|
||
According to the man page [1], sys_sysfs is obsolete and systems with /proc should use /proc/filesystems instead.
[1] http://man7.org/linux/man-pages/man2/sysfs.2.html
Assignee | ||
Comment 2•8 years ago
|
||
0x78 is 120 which is (32-bit) clone; I'm not seeing anything for 0x87 which would be sysfs.
Assignee | ||
Comment 3•8 years ago
|
||
%ebx is the first argument, which is the clone flags, which in all three is 0x01200011 == SIGCHLD | CLONE_CHILD_CLEARTID | CLONE_CHILD_SETTID. So it's a fork().
Assignee | ||
Comment 4•8 years ago
|
||
There are a bunch of 64-bit clone crashes too: https://crash-stats.mozilla.com/search/?product=Firefox&reason=~SIGSYS&address=%3D0x38&_sort=-build_id&_sort=-date&_facets=cpu_arch&_facets=address&_columns=date&_columns=signature&_columns=product&_columns=version&_columns=build_id&_columns=platform#crash-reports
Looks like there isn't already a bug for these. Also, this is now the same summary as bug 1259508 but not the same underlying cause.
If we can't find the underying cause for this one reasonably quickly, we could try making fork() fail with EACCES or EPERM or something.
Summary: Seccomp sandbox violation: sys_sysfs called in content process of Firefox desktop → Seccomp sandbox violation: sys_clone called in content process of Firefox desktop
Reporter | ||
Comment 5•8 years ago
|
||
Thanks :jld, confused something somewhere.
Yes, I haven't filed bugs for the sys_clone violations yet, there are quite a lot of reports. I requested access to the minidumps so that I can start investigating.
Assignee | ||
Comment 6•8 years ago
|
||
I'm looking at https://crash-stats.mozilla.com/report/index/993f6b19-4ae9-49ca-b4b7-5a3572160712 (which is from Ubuntu Trusty, so I can just `apt-get download` packages on the VM I already have). I found debug symbols for glib, and the frames breakpad reports at the top look plausible:
0x000000000008c54b
fork_exec_with_pipes
/build/buildd/glib2.0-2.40.2/./glib/gspawn.c:1326
0x000000000008ce17
g_spawn_sync
/build/buildd/glib2.0-2.40.2/./glib/gspawn.c:277 (discriminator 3)
0x000000000006361a
g_shell_parse_argv
/build/buildd/glib2.0-2.40.2/./glib/gshell.c:681
0x000000000008d516
g_spawn_command_line_sync
/build/buildd/glib2.0-2.40.2/./glib/gspawn.c:721
And then it goes into liboxygen-gtk.so. I don't have symbols there, but I do have a disassembly; here are the first few reported addresses that are actually in .text (note that they're reported as one byte before the end of the call instruction, which is the convention for querying debug info for the state during a call):
000000000009a730 <_ZNK6Oxygen10QtSettings10runCommandERKSsRPc>:
...
9a744: e8 27 6f fb ff callq 51670 <g_spawn_command_line_sync@plt>
000000000009cff0 <_ZNK6Oxygen10QtSettings17kdeConfigPathListEv>:
...
9d04b: e8 e0 d6 ff ff callq 9a730 <_ZNK6Oxygen10QtSettings10runCommandERKSsRPc>
00000000000a9ff0 <_ZN6Oxygen10QtSettings10initializeEj>:
...
aa0f2: e8 f9 2e ff ff callq 9cff0 <_ZNK6Oxygen10QtSettings17kdeConfigPathListEv>
Also looks plausible. A little poking around in Oxygen::QtSettings::kdeConfigPathList() shows it passing a std::string it constructed from a string literal stored at 0x10908a (which shows up in the crash report, offset by -1, because breakpad saw it on the stack but didn't know that it's .rodata and not .text). That string is:
00109080 6b 64 65 34 2d 63 | kde4-c|
00109090 6f 6e 66 69 67 20 2d 2d 70 61 74 68 20 63 6f 6e |onfig --path con|
001090a0 66 69 67 00 |fig |
Reporter | ||
Comment 7•8 years ago
|
||
Thanks :jld for looking into this.
Looking at the oxygen-gtk source [1], it shows that the failure of runCommand() isn't fatal and it has fallback code. So returning an error for fork() instead of crash would let the content process keep running.
[1] https://lxr.kde.org/source/playground/artwork/oxygen-gtk/src/oxygenqtsettings.cpp#0358
Updated•8 years ago
|
Whiteboard: sblc1
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → jld
Status: NEW → ASSIGNED
Assignee | ||
Comment 9•8 years ago
|
||
Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=399d1d678c2e
Tested with GTK3 Oxygen theme and SCIM (two fork() users I saw in crash reports), which both seem to work. Also verified with gdb that GMP child processes still crash on fork() as expected.
A related thing from the sandboxing meeting this morning: while we initialize the widget toolkit in the child process, native widget rendering (e.g., form fields) happens in the parent process. Hopefully this means there's more room for widget-related things not working quite right in the content process due to sandboxing, but I won't claim to know the graphics stack well enough to say anything definitive.
A possible followup: try to gather telemetry on how much we reject clone() calls like this.
Attachment #8771141 -
Flags: review?(julian.r.hector)
Assignee | ||
Updated•8 years ago
|
Crash Signature: [@ libc-2.19.so@0xb6e74 ] [@ libc-2.20.so@0xbd306 ] [@ libc-2.19.so@0xba014 ] [@ libc-2.19.so@0xc0ee4 ] [@ libc-2.22.so@0xb8a96 ]
Assignee | ||
Updated•8 years ago
|
Crash Signature: [@ libc-2.19.so@0xb6e74 ] [@ libc-2.20.so@0xbd306 ] [@ libc-2.19.so@0xba014 ] [@ libc-2.19.so@0xc0ee4 ] [@ libc-2.22.so@0xb8a96 ] → [@ libc-2.19.so@0xb6e74 ] [@ libc-2.20.so@0xbd306 ] [@ libc-2.19.so@0xba014 ] [@ libc-2.19.so@0xc0ee4 ] [@ libc-2.22.so@0xb8a96 ] [@ libpthread-2.19.so@0x10e93 ] [@ libpthread-2.19.so@0x10ed3 ]
Assignee | ||
Updated•8 years ago
|
Crash Signature: [@ libc-2.19.so@0xb6e74 ] [@ libc-2.20.so@0xbd306 ] [@ libc-2.19.so@0xba014 ] [@ libc-2.19.so@0xc0ee4 ] [@ libc-2.22.so@0xb8a96 ] [@ libpthread-2.19.so@0x10e93 ] [@ libpthread-2.19.so@0x10ed3 ] → [@ libc-2.19.so@0xb6e74 ] [@ libc-2.20.so@0xbd306 ] [@ libc-2.19.so@0xba014 ] [@ libc-2.19.so@0xc0ee4 ] [@ libc-2.22.so@0xb8a96 ] [@ libpthread-2.19.so@0x10e93 ] [@ libpthread-2.19.so@0x10ed3 ] [@ libc-2.23.so@0xb6f9a ]
Reporter | ||
Comment 10•8 years ago
|
||
Comment on attachment 8771141 [details] [diff] [review]
Patch: change forbidden clones to fail with EPERM
Review of attachment 8771141 [details] [diff] [review]:
-----------------------------------------------------------------
LGTM. From what I saw, some of the code has fallback code in case fork() fails with an error. But getting telemetry would be nice.
Attachment #8771141 -
Flags: review?(julian.r.hector) → review+
Comment 12•8 years ago
|
||
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7b3f1cc706bd
Make fork() non-fatal in Linux content sandbox. r=jhector
Keywords: checkin-needed
Comment 13•8 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox50:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
You need to log in
before you can comment on or make changes to this bug.
Description
•