967132 - Faulty crash: Assertion failure: aPid == base::GetProcId(mChildProcessHandle) in GeckoChildProcessHost::OpenPrivilegedHandle

Status: NEW

(Core :: IPC, defect)

Description

[Benoit Jacob [:bjacob] (mostly away)]

Attachment: Faulty session

Found by Christoph Diehl's "Faulty" fuzzer, see bug 777067

This is another case of "bad child process triggers an assertion failure in parent process", similar to bug 963978. Similar to this bug, it might not be a real-world problem per se, but we at least need a way to have this kind of issue not prevent further fuzzing.

Comment 1

[Benoit Jacob [:bjacob] (mostly away)]

Actually, this looks like a more clear-cut bug:

Code at ipc/glue/GeckoChildProcessHost.cpp:821:

817     void
818     GeckoChildProcessHost::OpenPrivilegedHandle(base::ProcessId aPid)
819     {
820       if (mChildProcessHandle) {
821         MOZ_ASSERT(aPid == base::GetProcId(mChildProcessHandle));
822         return;
823       }
824       if (!base::OpenPrivilegedProcessHandle(aPid, &mChildProcessHandle)) {
825         NS_RUNTIMEABORT("can't open handle to child process");
826       }
827     }


If I understand correctly, this OpenPrivilegedHandle function is "infallible" in that it hard-asserts, even in non-DEBUG builds, that base::OpenPrivilegedProcessHandle succeeds. But, on platforms where this function is non-trivial (like, Windows), this will fail if the child process just crashed, right? So this seems like something that should be handled gracefully, rather than "infallible". Is that correct?

Comment 2

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Yeah, parent failures should be graceful whenever it's reasonable to expect failure in the wild (at least imho). It may involve restructuring this code a bit though...

Comment 3

[Benoit Jacob [:bjacob] (mostly away)]

Non-gfx, unblocking gfx tracking bug.

Comment 4

[Benoit Jacob [:bjacob] (mostly away)]

Actually, no, I won't be able to get successful runs with Faulty until that is fixed.

Classification: non-gfx, hard (because of comment 2 "It may involve restructuring this code a bit though").