1438401 - There's still SysV shared memory usage from libcairo sometimes

Status: RESOLVED FIXED

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

Description

[Jed Davis [:jld] ⟨⏰|UTC-8⟩ ⟦he/him⟧]

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.)

Comment 1

[Jed Davis [:jld] ⟨⏰|UTC-8⟩ ⟦he/him⟧]

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.

Comment 2

[Jed Davis [:jld] ⟨⏰|UTC-8⟩ ⟦he/him⟧]

Attachment: Bug 1438401 - Quietly fail shmget() in sandboxed content processes.

The X11 symbol interposition isn't enough, possibly because Cairo can
also use XCB.  Interposing XCB is more difficult because the API exposes
more protocol details.  Instead, just allow shmget to be called and
fail; this will tell Cairo that it can't use SysV IPC with the X server,
which is what we want.

Review commit: https://reviewboard.mozilla.org/r/223732/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/223732/

Comment 3

[Gian-Carlo Pascutto [:gcp]]

Comment on attachment 8954649 [details]
Bug 1438401 - Quietly fail shmget() in sandboxed content processes.

https://reviewboard.mozilla.org/r/223732/#review229800

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/936b73ae6e3c
Quietly fail shmget() in sandboxed content processes. r=gcp

Comment 5

[Tiberius Oros[:tiberius_oros]]

https://hg.mozilla.org/mozilla-central/rev/936b73ae6e3c

Comment 6

[Guillaume Maudoux [:layus]]

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

Comment 7

[Jed Davis [:jld] ⟨⏰|UTC-8⟩ ⟦he/him⟧]

Thanks for pointing that out; I noticed it after I committed but I forgot to file a bug.