1068410 - Convert multiprocess crash reporter to use pipe() instead of socketpair() in the child

Status: RESOLVED FIXED

(Toolkit :: Crash Reporting, defect)

Description

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

See bug 1066750 for background.  The only message the parent needs to send back to the child is “done dumping”; a pipe works as well as a socket pair for that, and socketpair() has some unwanted features to which we'd like to deny access (which, for unfortunate historical reasons, means blocking all use of socketpair, at least on 32-bit x86).

The FindProcessHoldingSocket code in crash_generation_server has a number of assumptions about socketness, but we can probably remove all of it.  https://crbug.com/357670 covers the removal of the corresponding code from Chromium.  Basically, it's necessary only if a project is doing all of:

1. Using breakpad's remote dumping facility.  (This might only be Gecko, but I think we're not 100% sure of that.)
2. Supporting Linux kernels older than 2.6.35 (released August 2010).
3. Dumping descendent processes in different PID namespaces (e.g., using the Chromium sandbox); note that the affected kernels have recently been desupported by Chromium itself (see link above) and require either running as root or using a setuid-root helper to access this functionality.

We suspect that no non-Gecko projects satisfy all of the above conditions.  B2G could qualify, if we're still using the ICS emulator (kernel 2.6.29, and the only pre-3.0 B2G device) for continuous integration when the process-separation situation improves enough to consider using the full Chromium sandbox — but we control the kernel and can backport the fix, and the B2G sandboxing module owner (namely, me) is fine with that.

Comment 1

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

Attachment: Step 1: remove procfs-searching hack.

Upstream issue: https://breakpad.appspot.com/7724002/

Comment 2

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

Attachment: Step 2: switch crash report client to calling pipe().

Upstream issue for the breakpad part: https://breakpad.appspot.com/1724002/

This also adds pipe(2) to the GMP whitelist and un-conditionalizes it for content processes, so that that will work.  It removes socketpair(2) from the GMP whitelist only, because that syscall has other uses in content processes (see bug 1066750 comment #3).

This could be split into separate patches (allow pipe, change breakpad, disallow socketpair) if need be, but I don't know that that would be useful.

Comment 3

[(not currently active) Ted Mielczarek]

Comment on attachment 8496373 [details] [diff] [review]
Step 1: remove procfs-searching hack.

Review of attachment 8496373 [details] [diff] [review]:
-----------------------------------------------------------------

Patches that do nothing but remove code are pretty nice. :)

Comment 4

[(not currently active) Ted Mielczarek]

Comment on attachment 8496375 [details] [diff] [review]
Step 2: switch crash report client to calling pipe().

Review of attachment 8496375 [details] [diff] [review]:
-----------------------------------------------------------------

Boy, that's a remarkably small patch.

Comment 5

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

Attachment: Step 1: remove procfs-searching hack. [v2]

Updated with patch for breakpad-patches.  Carrying over r+.

Comment 6

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

Attachment: Step 2: switch crash report client to calling pipe(). [v2]

Updated with patch for breakpad-patches.  Carrying over r+.

Thanks for the reviews.

Comment 7

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/958697c700ca
https://hg.mozilla.org/integration/mozilla-inbound/rev/afeff2d265bd

Comment 8

[(not currently active) Ted Mielczarek]

I'll land the Breakpad patches upstream in a bit. ni? myself so I don't forget.

Comment 9

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/958697c700ca
https://hg.mozilla.org/mozilla-central/rev/afeff2d265bd

Comment 10

[(not currently active) Ted Mielczarek]

Landed upstream:
https://code.google.com/p/google-breakpad/source/detail?r=1389
https://code.google.com/p/google-breakpad/source/detail?r=1390

If you could close those breakpad.appspot.com issues that'd be great.