Closed Bug 1673525 Opened 4 years ago Closed 4 years ago

UndefinedBehaviorSanitizer: /builds/worker/workspace/obj-build/dist/include/SharedBuffer.h:58:30: runtime error: applying non-zero offset 16 to null pointer

Categories

(Core :: Audio/Video, defect, P2)

defect

Tracking

()

VERIFIED FIXED
86 Branch
Tracking Status
firefox-esr78 --- unaffected
firefox82 --- unaffected
firefox83 --- wontfix
firefox84 --- wontfix
firefox85 --- wontfix
firefox86 --- verified

People

(Reporter: jkratzer, Assigned: bryce)

References

(Blocks 1 open bug, Regression)

Details

(Keywords: assertion, regression, testcase, Whiteboard: [bugmon:confirmed,bisected])

Attachments

(3 files)

Attached file testcase.zip (deleted) —

Testcase found while fuzzing mozilla-central rev b1a74943bc51 (built with --enable-debug).

/builds/worker/workspace/obj-build/dist/include/SharedBuffer.h:58:30: runtime error: applying non-zero offset 16 to null pointer

    #0 0x7f442ba20760 in Data /builds/worker/workspace/obj-build/dist/include/SharedBuffer.h:58:30
    #1 0x7f442ba20760 in SendStreamAudio /builds/worker/checkouts/gecko/dom/media/mediasink/DecodedStream.cpp:627:69
    #2 0x7f442ba20760 in mozilla::DecodedStream::SendAudio(double, nsMainThreadPtrHandle<nsIPrincipal> const&) /builds/worker/checkouts/gecko/dom/media/mediasink/DecodedStream.cpp:663:5
    #3 0x7f442ba1d0e6 in mozilla::DecodedStream::SendData() /builds/worker/checkouts/gecko/dom/media/mediasink/DecodedStream.cpp:923:3
    #4 0x7f442ba1b2b3 in mozilla::DecodedStream::Start(mozilla::media::TimeUnit const&, mozilla::MediaInfo const&) /builds/worker/checkouts/gecko/dom/media/mediasink/DecodedStream.cpp:500:5
    #5 0x7f442ba28192 in mozilla::VideoSink::Start(mozilla::media::TimeUnit const&, mozilla::MediaInfo const&) /builds/worker/checkouts/gecko/dom/media/mediasink/VideoSink.cpp:217:29
    #6 0x7f442b43951b in mozilla::MediaDecoderStateMachine::StartMediaSink() /builds/worker/checkouts/gecko/dom/media/MediaDecoderStateMachine.cpp:3364:29
    #7 0x7f442b4235a4 in mozilla::MediaDecoderStateMachine::MaybeStartPlayback() /builds/worker/checkouts/gecko/dom/media/MediaDecoderStateMachine.cpp:2966:3
    #8 0x7f442b5c720f in mozilla::MediaDecoderStateMachine::CompletedState::Step() /builds/worker/checkouts/gecko/dom/media/MediaDecoderStateMachine.cpp:2008:16
    #9 0x7f442b5c844f in mozilla::MediaDecoderStateMachine::CompletedState::Enter() /builds/worker/checkouts/gecko/dom/media/MediaDecoderStateMachine.cpp:1991:5
    #10 0x7f442b4218a1 in CallEnterMemberFunction<mozilla::MediaDecoderStateMachine::CompletedState> /builds/worker/checkouts/gecko/dom/media/MediaDecoderStateMachine.cpp:265:16
    #11 0x7f442b4218a1 in decltype(ReturnTypeHelper(&(mozilla::MediaDecoderStateMachine::CompletedState::Enter))) mozilla::MediaDecoderStateMachine::StateObject::SetState<mozilla::MediaDecoderStateMachine::CompletedState>() /builds/worker/checkouts/gecko/dom/media/MediaDecoderStateMachine.cpp:306:12
    #12 0x7f442b5ed0e6 in mozilla::MediaDecoderStateMachine::RequestAudioData()::$_22::operator()(mozilla::MediaResult const&) const /builds/worker/checkouts/gecko/dom/media/MediaDecoderStateMachine.cpp
    #13 0x7f442b5eb8c4 in InvokeMethod<(lambda at /builds/worker/checkouts/gecko/dom/media/MediaDecoderStateMachine.cpp:3229:11), void ((lambda at /builds/worker/checkouts/gecko/dom/media/MediaDecoderStateMachine.cpp:3229:11)::*)(const mozilla::MediaResult &) const, mozilla::MediaResult> /builds/worker/workspace/obj-build/dist/include/mozilla/MozPromise.h:553:12
    #14 0x7f442b5eb8c4 in InvokeCallbackMethod<false, (lambda at /builds/worker/checkouts/gecko/dom/media/MediaDecoderStateMachine.cpp:3229:11), void ((lambda at /builds/worker/checkouts/gecko/dom/media/MediaDecoderStateMachine.cpp:3229:11)::*)(const mozilla::MediaResult &) const, mozilla::MediaResult, RefPtr<mozilla::MozPromise<RefPtr<mozilla::AudioData>, mozilla::MediaResult, true>::Private> > /builds/worker/workspace/obj-build/dist/include/mozilla/MozPromise.h:584:5
    #15 0x7f442b5eb8c4 in mozilla::MozPromise<RefPtr<mozilla::AudioData>, mozilla::MediaResult, true>::ThenValue<mozilla::MediaDecoderStateMachine::RequestAudioData()::$_21, mozilla::MediaDecoderStateMachine::RequestAudioData()::$_22>::DoResolveOrRejectInternal(mozilla::MozPromise<RefPtr<mozilla::AudioData>, mozilla::MediaResult, true>::ResolveOrRejectValue&) /builds/worker/workspace/obj-build/dist/include/mozilla/MozPromise.h:773:9
    #16 0x7f442b5c1c4e in mozilla::MozPromise<RefPtr<mozilla::AudioData>, mozilla::MediaResult, true>::ThenValueBase::ResolveOrRejectRunnable::Run() /builds/worker/workspace/obj-build/dist/include/mozilla/MozPromise.h:410:21
    #17 0x7f442522b9c0 in mozilla::AutoTaskDispatcher::TaskGroupRunnable::Run() /builds/worker/workspace/obj-build/dist/include/mozilla/TaskDispatcher.h:228:35
    #18 0x7f44252348c5 in mozilla::TaskQueue::Runner::Run() /builds/worker/checkouts/gecko/xpcom/threads/TaskQueue.cpp:158:20
    #19 0x7f442525f1ca in nsThreadPool::Run() /builds/worker/checkouts/gecko/xpcom/threads/nsThreadPool.cpp:299:14
    #20 0x7f4425250467 in nsThread::ProcessNextEvent(bool, bool*) /builds/worker/checkouts/gecko/xpcom/threads/nsThread.cpp:1197:14
    #21 0x7f442525affc in NS_ProcessNextEvent(nsIThread*, bool) /builds/worker/checkouts/gecko/xpcom/threads/nsThreadUtils.cpp:513:10
    #22 0x7f4426534f22 in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) /builds/worker/checkouts/gecko/ipc/glue/MessagePump.cpp:302:20
    #23 0x7f4426435641 in RunInternal /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:334:10
    #24 0x7f4426435641 in RunHandler /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:327:3
    #25 0x7f4426435641 in MessageLoop::Run() /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:309:3
    #26 0x7f4425249572 in nsThread::ThreadFunc(void*) /builds/worker/checkouts/gecko/xpcom/threads/nsThread.cpp:442:10
    #27 0x7f443e69842e in _pt_root /builds/worker/checkouts/gecko/nsprpub/pr/src/pthreads/ptthread.c:201:5
    #28 0x7f4441f90608 in start_thread /build/glibc-ZN95T4/glibc-2.31/nptl/pthread_create.c:477:8
    #29 0x7f4441b59292 in clone /build/glibc-ZN95T4/glibc-2.31/misc/../sysdeps/unix/sysv/linux/x86_64/clone.S:95
Flags: in-testsuite?

The bug appears to have been introduced in the following build range:

Start: 2a46f29c8cdd6f6269483e86c6e21e649ffa7127 (20201020234416)
End: 4c3e6fb95aa6184651f215d466e702ff7d1660d3 (20201021000031)
Pushlog: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=2a46f29c8cdd6f6269483e86c6e21e649ffa7127&tochange=4c3e6fb95aa6184651f215d466e702ff7d1660d3

Whiteboard: [bugmon:confirm] → [bugmon:confirmed,bisected]
Regressed by: 1595994

Set release status flags based on info from the regressing bug 1595994

Work is currently kept in nightly.

Severity: normal → S2
Flags: needinfo?(bvandyk)

When testing this it looks like the major important pref to set/action to take is to allow autoplay on the page. I tested this using debug builds which I believe trigger asserts due to null ptrs being involved.

After looking at this for a bit I re-bisected due to being unable to find the issue in bug 1595994. I believe bug 1566389 may actually be our regressor. Specifically https://phabricator.services.mozilla.com/D91773 , I verified locally via hg backout bc144d4e01d0 after which I no longer hit asserts.

Bug investigation:

When we don't have any audio data the logic in decoded stream will get a null ptr when we try and get our buffer[0] and the UB results from that.

I don't know this code well, and it makes references to mimicking other code that seems likely to have changed since this was written (e.g. this code references DecodedAudioDataSink, which no longer exists).

It looks like the code is supposed to push silence to fill gaps and then push data after that silence. I wonder if it's just missing an edge case, or if there's some greater issue here. Will keep looking.

[0] https://searchfox.org/mozilla-central/rev/23c25cd32a1e87095301273937b4ee162f41e860/dom/media/mediasink/DecodedStream.cpp#625

Flags: needinfo?(bvandyk)
Regressed by: 1566389
No longer regressed by: 1595994
Has Regression Range: --- → yes

Updating flags based on adjusted regressor.

(In reply to Jean-Yves Avenard [:jya] from comment #7)

Wouldn't https://bugzilla.mozilla.org/show_bug.cgi?id=1669503 be a fix for this?

I'll believe it. I'm not familiar with various machinery between the demux and here, but I'd expect since mp3's made this unhappy it could be fixed there too.

It looks like this code is doing something similar to https://searchfox.org/mozilla-central/rev/38ed718a101aca27db25984413c052ccd8c0ceda/dom/media/mediasink/AudioSink.cpp#320 and historically I guess it was going for something like https://searchfox.org/mozilla-central/rev/74504dc15aad2b850911040a3cd8041e4faf4c79/dom/media/mediasink/DecodedAudioDataSink.cpp#396

If the intention is to do the same thing, I wonder if we can abstract out that logic. However, since I don't know the code I don't want to spend time working on something to find out we'd prefer another solution or that his is entirely a problem elsewhere. I've got a band aid for this I'll stick up + a crashtest for this.

NI Paul for comment on the mp3 changes.

Assignee: nobody → bvandyk
Severity: S2 → S3
Flags: needinfo?(padenot)

(In reply to Bryce Seager van Dyk (:bryce) from comment #8)

If the intention is to do the same thing, I wonder if we can abstract out that logic. However, since I don't know the code I don't want to spend time working on something to find out we'd prefer another solution or that his is entirely a problem elsewhere. I've got a band aid for this I'll stick up + a crashtest for this.

It is. Long-term we want to route audio from playback via the graph so that we don't need two separate sinks. Intermediate-term we might want to use one sink for both audio backends, probably as part of shipping captureStream, that would also de-dupe this code.

Pushed by bvandyk@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/d0221fa8b967 Add crashtest. r=pehrsons https://hg.mozilla.org/integration/autoland/rev/cf453005952e Don't try and process audio with no frames in DecodedStream. r=pehrsons
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → 86 Branch

Bugmon Analysis:
Verified bug as fixed on rev mozilla-central 20210112215931-bdc025488805.
Removing bugmon keyword as no further action possible.
Please review the bug and re-add the keyword for further analysis.

Status: RESOLVED → VERIFIED
Keywords: bugmon
Flags: needinfo?(padenot)
Flags: in-testsuite?
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: