1351553 - Divide by zero in [@ mozilla::MediaDecoder::ComputePlaybackRate]

Status: RESOLVED FIXED

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

Description

[Tyson Smith [:tsmith]]

Attachment: test_case.mp4

This was uncovered using Undefined behavior sanitizer (UBSan) so it may not trigger a crash.

/home/user/code/mozilla-central/dom/media/MediaDecoder.cpp:1042:38: runtime error: division by zero
    #0 0x7f41017bb3b1 in mozilla::MediaDecoder::ComputePlaybackRate() /home/user/code/mozilla-central/dom/media/MediaDecoder.cpp:1042:38
    #1 0x7f41017c1070 in mozilla::MediaDecoder::UpdatePlaybackRate() /home/user/code/mozilla-central/dom/media/MediaDecoder.cpp:1057:3
    #2 0x7f41017b7ac9 in mozilla::MediaDecoder::DurationChanged() /home/user/code/mozilla-central/dom/media/MediaDecoder.cpp:1250:3
    #3 0x7f4101859fbc in mozilla::WatchManager<mozilla::MediaDecoder>::PerCallbackWatcher::DoNotify() /home/user/code/mozilla-central/objdir-ff-ubsan/dist/include/mozilla/StateWatching.h:276:9
    #4 0x7f410185a87d in mozilla::detail::RunnableMethodImpl<mozilla::WatchManager<mozilla::MediaDecoder>::PerCallbackWatcher*, void (mozilla::WatchManager<mozilla::MediaDecoder>::PerCallbackWatcher::*)(), true, false>::Run() /home/user/code/mozilla-central/objdir-ff-ubsan/dist/include/nsThreadUtils.h:899:13
    #5 0x7f40fc614b04 in mozilla::AutoTaskDispatcher::DrainDirectTasks() /home/user/code/mozilla-central/objdir-ff-ubsan/dist/include/mozilla/TaskDispatcher.h:105:10
    #6 0x7f40fc61574e in mozilla::AutoTaskDispatcher::TaskGroupRunnable::Run() /home/user/code/mozilla-central/objdir-ff-ubsan/dist/include/mozilla/TaskDispatcher.h:198:9
    #7 0x7f40fc612fa3 in mozilla::EventTargetWrapper::Runner::Run() /home/user/code/mozilla-central/xpcom/threads/AbstractThread.cpp:166:32
    #8 0x7f40fc63cba4 in nsThread::ProcessNextEvent(bool, bool*) /home/user/code/mozilla-central/xpcom/threads/nsThread.cpp:1269:14
    #9 0x7f40fc639c60 in NS_ProcessNextEvent(nsIThread*, bool) /home/user/code/mozilla-central/xpcom/threads/nsThreadUtils.cpp:389:10
    #10 0x7f40fd6a9ec4 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /home/user/code/mozilla-central/ipc/glue/MessagePump.cpp:96:21
    #11 0x7f40fd5512df in MessageLoop::Run() /home/user/code/mozilla-central/ipc/chromium/src/base/message_loop.cc:211:3
    #12 0x7f4102b1f235 in nsBaseAppShell::Run() /home/user/code/mozilla-central/widget/nsBaseAppShell.cpp:156:27
    #13 0x7f41069a3e6c in nsAppStartup::Run() /home/user/code/mozilla-central/toolkit/components/startup/nsAppStartup.cpp:283:30
    #14 0x7f4106b431e3 in XREMain::XRE_mainRun() /home/user/code/mozilla-central/toolkit/xre/nsAppRunner.cpp:4512:22
    #15 0x7f4106b44dd2 in XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) /home/user/code/mozilla-central/toolkit/xre/nsAppRunner.cpp:4692:8
    #16 0x7f4106b45d32 in XRE_main(int, char**, mozilla::BootstrapConfig const&) /home/user/code/mozilla-central/toolkit/xre/nsAppRunner.cpp:4783:21
    #17 0x4edd13 in do_main(int, char**, char**) /home/user/code/mozilla-central/browser/app/nsBrowserApp.cpp:236:22
    #18 0x4ed565 in main /home/user/code/mozilla-central/browser/app/nsBrowserApp.cpp:307:16
    #19 0x7f4122a8382f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)
    #20 0x41f378 in _start (/home/user/workspace/browsers/firefox_ub/firefox+0x41f378)

Comment 1

[JW Wang [:jwwang] [:jw_wang]]

Attachment: Bug 1351553 - fix divide-by-zero.

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

Comment 2

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8852372 [details]
Bug 1351553 - fix divide-by-zero.

https://reviewboard.mozilla.org/r/124612/#review127154

r+ with optional suggestion:

::: dom/media/MediaDecoder.cpp:1039
(Diff revision 1)
>  {
>    MOZ_ASSERT(NS_IsMainThread());
>    MOZ_ASSERT(mResource);
>  
>    int64_t length = mResource->GetLength();
> -  if (!IsNaN(mDuration) && !mozilla::IsInfinite<double>(mDuration)
> +  if (!mozilla::IsInfinite<double>(mDuration)

Instead of !IsInfinite, you could do IsFinite, which filters both infinity and NaN.
(I'm guessing `mDuration > 0` would be false with NaN, but I think using IsFinite would be clearer anyway.)

Comment 3

[JW Wang [:jwwang] [:jw_wang]]

Thanks for the suggestion!

Comment 4

[JW Wang [:jwwang] [:jw_wang]]

Comment on attachment 8852372 [details]
Bug 1351553 - fix divide-by-zero.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/124612/diff/1-2/

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/68304e449ef7
fix divide-by-zero. r=gerald

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/68304e449ef7