Closed Bug 1286324 Opened 3 years ago Closed 3 years ago

Seccomp sandbox violation: sys_clone called in content process of Firefox desktop


(Core :: Security: Process Sandboxing, defect)

Not set



Tracking Status
firefox50 --- fixed


(Reporter: tedd, Assigned: jld)



(Whiteboard: sblc1)

Crash Data


(1 file)

According to the man page [1], sys_sysfs is obsolete and systems with /proc should use /proc/filesystems instead.

0x78 is 120 which is (32-bit) clone; I'm not seeing anything for 0x87 which would be sysfs.
%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().
There are a bunch of 64-bit clone crashes too:

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
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.
I'm looking at (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:

/build/buildd/glib2.0-2.40.2/./glib/gspawn.c:277 (discriminator 3)

And then it goes into  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             |
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.

Whiteboard: sblc1
Assignee: nobody → jld
Duplicate of this bug: 1285637

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)
Crash Signature: [@ ] [@ ] [@ ] [@ ] [@ ]
Crash Signature: [@ ] [@ ] [@ ] [@ ] [@ ] → [@ ] [@ ] [@ ] [@ ] [@ ] [@ ] [@ ]
Crash Signature: [@ ] [@ ] [@ ] [@ ] [@ ] [@ ] [@ ] → [@ ] [@ ] [@ ] [@ ] [@ ] [@ ] [@ ] [@ ]
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+
Pushed by
Make fork() non-fatal in Linux content sandbox. r=jhector
Keywords: checkin-needed
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
Depends on: 1287437
You need to log in before you can comment on or make changes to this bug.