1381934 - IPDL error [PExternalHelperAppChild]: "NULL actor value passed to non-nullable param"

Status: RESOLVED FIXED

(Core :: IPC, defect)

Description

[Christoph Diehl [:posidron]]

Attachment: 66f3556c_2017-07-18_10-35-41_log.txt

Fuzzer: Message Manager


Please see the attached log for a detailed session overview. The test-case package contains the last 4 test-cases being used for the crash occurred.


IPDL protocol error: NULL actor value passed to non-nullable param
[Child 30409] ###!!! ABORT: IPDL error [PExternalHelperAppChild]: "NULL actor value passed to non-nullable param". abort()ing as a result.: file /home/worker/workspace/build/src/ipc/glue/ProtocolUtils.cpp, line 306
[Child 30409] ###!!! ABORT: IPDL error [PExternalHelperAppChild]: "NULL actor value passed to non-nullable param". abort()ing as a result.: file /home/worker/workspace/build/src/ipc/glue/ProtocolUtils.cpp, line 306
ASAN:DEADLYSIGNAL
=================================================================
==30409==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x0000004faa17 bp 0x7ffc507de390 sp 0x7ffc507de380 T0)
==30409==The signal is caused by a WRITE memory access.
==30409==Hint: address points to the zero page.
    #0 0x4faa16 in mozalloc_abort(char const*) /home/worker/workspace/build/src/memory/mozalloc/mozalloc_abort.cpp:33:5
    #1 0x7f678fea19f1 in NS_DebugBreak /home/worker/workspace/build/src/xpcom/base/nsDebugImpl.cpp
    #2 0x7f6790eb8ace in mozilla::ipc::FatalError(char const*, char const*, bool) /home/worker/workspace/build/src/ipc/glue/ProtocolUtils.cpp:306:5
    #3 0x7f679184aee6 in Write /home/worker/workspace/build/src/obj-firefox/ipc/ipdl/PExternalHelperAppChild.cpp:317:13
    #4 0x7f679184aee6 in mozilla::dom::PExternalHelperAppChild::SendDivertToParentUsing(mozilla::net::PChannelDiverterChild*, mozilla::dom::PBrowserChild*) /home/worker/workspace/build/src/obj-firefox/ipc/ipdl/PExternalHelperAppChild.cpp:132
    #5 0x7f6791c86353 in mozilla::dom::ExternalHelperAppChild::DivertToParent(nsIDivertableChannel*, nsIRequest*, mozilla::dom::TabChild*) /home/worker/workspace/build/src/uriloader/exthandler/ExternalHelperAppChild.cpp:120:7
    #6 0x7f6791c83f98 in mozilla::dom::ExternalHelperAppChild::OnStartRequest(nsIRequest*, nsISupports*) /home/worker/workspace/build/src/uriloader/exthandler/ExternalHelperAppChild.cpp:77:12
    #7 0x7f6791c61d97 in nsDocumentOpenInfo::OnStartRequest(nsIRequest*, nsISupports*) /home/worker/workspace/build/src/uriloader/base/nsURILoader.cpp:307:34
    #8 0x7f6790a3b7e8 in mozilla::net::HttpChannelChild::DoOnStartRequest(nsIRequest*, nsISupports*) /home/worker/workspace/build/src/netwerk/protocol/http/HttpChannelChild.cpp:667:28
    #9 0x7f6790a473e3 in mozilla::net::HttpChannelChild::OnStartRequest(nsresult const&, mozilla::net::nsHttpResponseHead const&, bool const&, mozilla::net::nsHttpHeaderArray const&, bool const&, bool const&, int const&, unsigned int const&, unsigned int const&, nsCString const&, nsCString const&, mozilla::net::NetAddr const&, mozilla::net::NetAddr const&, unsigned int const&, nsCString const&, long const&) /home/worker/workspace/build/src/netwerk/protocol/http/HttpChannelChild.cpp:598:3
    #10 0x7f6790b2be1c in mozilla::net::StartRequestEvent::Run() /home/worker/workspace/build/src/netwerk/protocol/http/HttpChannelChild.cpp:430:13
    #11 0x7f679091784f in mozilla::net::ChannelEventQueue::RunOrEnqueue(mozilla::net::ChannelEvent*, bool) /home/worker/workspace/build/src/obj-firefox/dist/include/mozilla/net/ChannelEventQueue.h:215:10
    #12 0x7f6790a463d2 in mozilla::net::HttpChannelChild::RecvOnStartRequest(nsresult const&, mozilla::net::nsHttpResponseHead const&, bool const&, mozilla::net::nsHttpHeaderArray const&, bool const&, bool const&, int const&, unsigned int const&, unsigned int const&, nsCString const&, nsCString const&, mozilla::net::NetAddr const&, mozilla::net::NetAddr const&, short const&, unsigned int const&, nsCString const&, long const&) /home/worker/workspace/build/src/netwerk/protocol/http/HttpChannelChild.cpp:487:12
    #13 0x7f67910931e8 in mozilla::net::PHttpChannelChild::OnMessageReceived(IPC::Message const&) /home/worker/workspace/build/src/obj-firefox/ipc/ipdl/PHttpChannelChild.cpp:637:20
    #14 0x7f67916c96a1 in mozilla::dom::PContentChild::OnMessageReceived(IPC::Message const&) /home/worker/workspace/build/src/obj-firefox/ipc/ipdl/PContentChild.cpp:5296:28
    #15 0x7f6790eae59e in mozilla::ipc::MessageChannel::DispatchAsyncMessage(IPC::Message const&) /home/worker/workspace/build/src/ipc/glue/MessageChannel.cpp:2093:25
    #16 0x7f6790eab8d9 in mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message&&) /home/worker/workspace/build/src/ipc/glue/MessageChannel.cpp:2019:17
    #17 0x7f6790eacfa4 in mozilla::ipc::MessageChannel::RunMessage(mozilla::ipc::MessageChannel::MessageTask&) /home/worker/workspace/build/src/ipc/glue/MessageChannel.cpp:1888:5
    #18 0x7f6790ead5a8 in mozilla::ipc::MessageChannel::MessageTask::Run() /home/worker/workspace/build/src/ipc/glue/MessageChannel.cpp:1921:15
    #19 0x7f6790010feb in mozilla::SchedulerGroup::Runnable::Run() /home/worker/workspace/build/src/xpcom/threads/SchedulerGroup.cpp:367:25

Comment 1

[Christoph Diehl [:posidron]]

Attachment: 2b215b1f0d2291dca17f096452a1607d4687bb00.zip

testcases

Comment 3

[Andrew Overholt [:overholt]]

Bill, is this an intentional crash?

Comment 4

[Andrew McCreight [:mccr8]]

This is an intentional crash, the question is, is there something we should do about this particular crash to try to avoid this if we're getting broken messages from the other process?

The crash looks like it is here:
    if ((!(v__))) {
        if ((!(nullable__))) {
            FatalError("NULL actor value passed to non-nullable param");
        }
So I think divertable->DivertToParent(&diverter) returned a success value, but diverter is null.

Comment 5

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Based on the line numbers, it looks like it's the TabChild that's null here:
http://searchfox.org/mozilla-central/rev/ad093e98f42338effe2e2513e26c3a311dd96422/uriloader/exthandler/ExternalHelperAppChild.cpp#120

That value comes from here:
http://searchfox.org/mozilla-central/rev/ad093e98f42338effe2e2513e26c3a311dd96422/uriloader/exthandler/ExternalHelperAppChild.cpp#73

Blake, should we be doing something different here? If we want to allow null to be passed over the channel, we need to declare the parameter as nullable in IPDL.

Comment 6

[Blake Kaplan (:mrbkap) (inactive)]

(In reply to Bill McCloskey (:billm) from comment #5)
> Blake, should we be doing something different here? If we want to allow null
> to be passed over the channel, we need to declare the parameter as nullable
> in IPDL.

I suspect that this is a dupe of bug 1368343. I wasn't able to reproduce the crash with the testcase here, but the stack shows some massively nested event loops and the log included in the zip shows that the harness is timing out (and closing) the test window. That suggests that we're closing the tab/window that we're running the test in and causing the GetInterface in OnStartRequest to return null.

Comment 7

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Bug 1381934 - Try to deal better with closed windows.

It's hard to know exactly what's going on in this testcase because we nest a
bunch of calls. I was unable to observe the crash myself, but it appears that
part of it (at least) has to do with the test timing out and causing the
opened page to get closed by the harness. I believe this causes us to fail to
getInterface to the docShell for the now-closed window and we crash.

There isn't much we can do in this situation so, let's try being defensive and
watch crash-stats.

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

Comment 8

[Christoph Diehl [:posidron]]

I forgot to mention that reproduction will only work with a --enable-fuzzing build, only those builds expose the MessageManager fuzzer. A possible reproduction scenario would be to get a --enable-fuzzing build from here:

https://tools.taskcluster.net/groups/X4xSKKzzSVyBzwMqTLg5DQ/tasks/V33s0QhtTmu3B6UomdE0QA/runs/0/artifacts

And set these environment variables:

MESSAGEMANAGER_FUZZER_ENABLE_LOGGING=1
MESSAGEMANAGER_FUZZER_ENABLE=1
MESSAGEMANAGER_FUZZER_MUTATION_PROBABILITY=4

Comment 9

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8891559 [details]
Bug 1381934 - Try to deal better with closed windows.

https://reviewboard.mozilla.org/r/162676/#review168492

Thanks!

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/713ec2a7dc6c
Try to deal better with closed windows. r=mconley

Comment 11

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/713ec2a7dc6c