Closed Bug 1438401 Opened 2 years ago Closed 2 years ago

There's still SysV shared memory usage from libcairo sometimes

Categories

(Core :: Security: Process Sandboxing, enhancement, P1)

60 Branch
Unspecified
Linux
enhancement

Tracking

()

RESOLVED FIXED
mozilla60
Tracking Status
firefox60 --- fixed

People

(Reporter: jld, Assigned: jld)

References

(Blocks 1 open bug)

Details

Crash Data

Attachments

(1 file)

I *thought* the libcairo use of SysV shared memory in content processes could be explained by libXext being preloaded and defeating the workaround from bug 1271100, but apparently not.

It's not completely certain that libcairo is involved — Breakpad "scan" frames just mean the address was in the stack memory for some reason — but that's the only other reported stack frame besides the one in libc in the thread that's shmget()ing.

Looking at the Cairo source… maybe we also need to intercept xcb_shm_query_version?  ltrace'ing a local Firefox, it always calls XShmQueryExtension, but libcairo seems to have both traditional Xlib and XCB backends compiled in; maybe the cases where this still happens, Cairo is built with only the XCB support.

(This is the system libcairo; we have an in-tree Cairo and no longer support --enable-system-cairo, but the system Cairo seems to be pulled in via GTK.)
Assignee: nobody → jld
Priority: -- → P1
Crash Signature: [@ libc-2.26.so@0xf7c10 ] → [@ libc-2.26.so@0xf7c10 ] [@ libc-2.25.90.so@0xeedb0 ]
The XCB interface is more complex than the Xlib interface; instead of just `return False;`, I'd have to fake a cookie (sequence number) and potentially an X11 error record.

So, another way to deal with this is to quietly fail shmget().  This will tell Cairo (both backends) not use use SysV shm even if the server advertises MIT-SHM — it probes the X server to see whether it's in the same SysV IPC context (rather than, e.g., a different host), and a failure at any step of the process will `return FALSE;`.  This should obviate the need for the XShmQueryExtention shim.
Crash Signature: [@ libc-2.26.so@0xf7c10 ] [@ libc-2.25.90.so@0xeedb0 ] → [@ libc-2.26.so@0xf7c10 ] [@ libc-2.25.90.so@0xeedb0 ] [@ libc-2.26.so@0x117dc0 ]
Crash Signature: [@ libc-2.26.so@0xf7c10 ] [@ libc-2.25.90.so@0xeedb0 ] [@ libc-2.26.so@0x117dc0 ] → [@ libc-2.26.so@0xf7c10 ] [@ libc-2.25.90.so@0xeedb0 ] [@ libc-2.26.so@0x117dc0 ] [@ libc-2.27.so@0x122a7a ]
Comment on attachment 8954649 [details]
Bug 1438401 - Quietly fail shmget() in sandboxed content processes.

https://reviewboard.mozilla.org/r/223732/#review229800
Attachment #8954649 - Flags: review?(gpascutto) → review+
Pushed by jedavis@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/936b73ae6e3c
Quietly fail shmget() in sandboxed content processes. r=gcp
https://hg.mozilla.org/mozilla-central/rev/936b73ae6e3c
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
Not too sure how  to report this, but this fix removed all the need of the X header in https://hg.mozilla.org/mozilla-central/file/936b73ae6e3c/security/sandbox/linux/SandboxHooks.cpp#l18 but still includes it.
It is only a partial revert of https://hg.mozilla.org/mozilla-central/rev/74b5e036363f#l2.1 which is the previous commit on /security/sandbox/linux/SandboxHooks.cpp
Flags: needinfo?(jld)
Thanks for pointing that out; I noticed it after I committed but I forgot to file a bug.
Flags: needinfo?(jld)
You need to log in before you can comment on or make changes to this bug.