1790526 - src/toolkit/components/sessionstore/SessionStoreParent.cpp:281:47: runtime error: member call on null pointer of type 'mozilla::dom::BrowsingContext'

Status: RESOLVED FIXED

(Core :: Storage: localStorage & sessionStorage, defect, P3)

Description

[Tyson Smith [:tsmith]]

Attachment: testcase.html

This was found by enabling the null check in UBSan and running attached tests.

To enable this check add the following to your mozconfig:

ac_add_options --enable-undefined-sanitizer="null"

src/toolkit/components/sessionstore/SessionStoreParent.cpp:281:47: runtime error: member call on null pointer of type 'mozilla::dom::BrowsingContext'
    #0 0x7f767a3bb1f3 in mozilla::dom::SessionStoreParent::RecvIncrementalSessionStoreUpdate(mozilla::dom::MaybeDiscarded<mozilla::dom::BrowsingContext> const&, mozilla::Maybe<mozilla::dom::sessionstore::FormData> const&, mozilla::Maybe<nsPoint> const&, unsigned int) src/toolkit/components/sessionstore/SessionStoreParent.cpp:281:47
    #1 0x7f767a3d0974 in mozilla::dom::PSessionStoreParent::OnMessageReceived(IPC::Message const&) src/objdir-ff-ubsan/ipc/ipdl/PSessionStoreParent.cpp:297:86
    #2 0x7f7673f2400b in mozilla::dom::PContentParent::OnMessageReceived(IPC::Message const&) src/objdir-ff-ubsan/ipc/ipdl/PContentParent.cpp:6621:32
    #3 0x7f766ccd1579 in mozilla::ipc::MessageChannel::DispatchAsyncMessage(mozilla::ipc::ActorLifecycleProxy*, IPC::Message const&) src/ipc/glue/MessageChannel.cpp:1756:25
    #4 0x7f766cccd8a1 in mozilla::ipc::MessageChannel::DispatchMessage(mozilla::ipc::ActorLifecycleProxy*, mozilla::UniquePtr<IPC::Message, mozilla::DefaultDelete<IPC::Message> >) src/ipc/glue/MessageChannel.cpp:1681:9
    #5 0x7f766ccce97a in mozilla::ipc::MessageChannel::RunMessage(mozilla::ipc::ActorLifecycleProxy*, mozilla::ipc::MessageChannel::MessageTask&) src/ipc/glue/MessageChannel.cpp:1481:3
    #6 0x7f766cccfd71 in mozilla::ipc::MessageChannel::MessageTask::Run() src/ipc/glue/MessageChannel.cpp:1579:14
    #7 0x7f766b31f60e in mozilla::RunnableTask::Run() src/xpcom/threads/TaskController.cpp:538:16
    #8 0x7f766b2d6d6f in mozilla::TaskController::DoExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) src/xpcom/threads/TaskController.cpp:851:26
    #9 0x7f766b2d3937 in mozilla::TaskController::ExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) src/xpcom/threads/TaskController.cpp:683:15
    #10 0x7f766b2d419b in mozilla::TaskController::ProcessPendingMTTask(bool) src/xpcom/threads/TaskController.cpp:461:36
    #11 0x7f766b30b079 in mozilla::TaskController::InitializeInternal()::$_0::operator()() const src/xpcom/threads/TaskController.cpp:187:37
    #12 0x7f766b30b079 in mozilla::detail::RunnableFunction<mozilla::TaskController::InitializeInternal()::$_0>::Run() src/objdir-ff-ubsan/dist/include/nsThreadUtils.h:531:5
    #13 0x7f766b2f50ba in nsThread::ProcessNextEvent(bool, bool*) src/xpcom/threads/nsThread.cpp:1205:16
    #14 0x7f766b2fe8d8 in NS_ProcessNextEvent(nsIThread*, bool) src/xpcom/threads/nsThreadUtils.cpp:465:10
    #15 0x7f766ccd9f7f in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:85:21
    #16 0x7f766cb2cc64 in MessageLoop::RunInternal() src/ipc/chromium/src/base/message_loop.cc:381:10
    #17 0x7f766cb2cc64 in MessageLoop::RunHandler() src/ipc/chromium/src/base/message_loop.cc:374:3
    #18 0x7f766cb2cc64 in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:356:3
    #19 0x7f7674b7e52c in nsBaseAppShell::Run() src/widget/nsBaseAppShell.cpp:150:27
    #20 0x7f767a3f471a in nsAppStartup::Run() src/toolkit/components/startup/nsAppStartup.cpp:295:30
    #21 0x7f767a65bb8b in XREMain::XRE_mainRun() src/toolkit/xre/nsAppRunner.cpp:5720:22
    #22 0x7f767a65dceb in XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) src/toolkit/xre/nsAppRunner.cpp:5913:8
    #23 0x7f767a65eacb in XRE_main(int, char**, mozilla::BootstrapConfig const&) src/toolkit/xre/nsAppRunner.cpp:5969:21
    #24 0x5632ed455c4e in do_main(int, char**, char**) src/browser/app/nsBrowserApp.cpp:226:22
    #25 0x5632ed454d0d in main src/browser/app/nsBrowserApp.cpp:430:16
    #26 0x7f7699030c86 in __libc_start_main /build/glibc-CVJwZb/glibc-2.27/csu/../csu/libc-start.c:310
    #27 0x5632ed394bd8 in _start (src/objdir-ff-ubsan/dist/bin/firefox+0x10abd8) (BuildId: 2b7f725cc2c422ad43d4caa97d841bcb74b001b7)

Comment 1

[Jens Stutte [:jstutte]]

Looks like a regression from bug 1739450 ?

Comment 2

[Jens Stutte [:jstutte]]

:hsingh, could you please check if this still reproduces?

Comment 3

[Eden Chuang[:edenchuang]]

I will check if this is reproducible.

Comment 4

[Tyson Smith [:tsmith]]

It is still reproducible with m-c df965ece56c1 from 2024-12-14.

Comment 5

[Eden Chuang[:edenchuang]]

After adding ac_add_options --enable-undefined-sanitizer="null" in MOZCONFIG file. I got another crash stacks

#0 0x00012bc3d718 in void mozilla::detail::CheckedUnsafePtrBase<mozilla::dom::WorkerPrivate, (mozilla::CheckingSupport)1>::Replace<mozilla::CheckedUnsafePtr<mozilla::dom::WorkerPrivate>>(mozilla::CheckedUnsafePtr<mozilla::dom::WorkerPrivate> const&)+0x100 (XUL:arm64+0x6d85718)
#1 0x00012bc34a78 in mozilla::dom::WorkerEventTarget::ForgetWorkerPrivate(mozilla::dom::WorkerPrivate*)+0xf4 (XUL:arm64+0x6d7ca78)
#2 0x00012bc5e1e8 in mozilla::dom::WorkerPrivate::RunShutdownTasks()+0xe0 (XUL:arm64+0x6da61e8)
#3 0x00012bc36434 in mozilla::dom::workerinternals::(anonymous namespace)::WorkerThreadPrimaryRunnable::Run()+0x390 (XUL:arm64+0x6d7e434)
#4 0x00012555072c in nsThread::ProcessNextEvent(bool, bool*)+0x5c8 (XUL:arm64+0x69872c)
#5 0x000125556ef8 in NS_ProcessNextEvent(nsIThread*, bool)+0x60 (XUL:arm64+0x69eef8)
#6 0x0001262c5cd8 in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*)+0xec (XUL:arm64+0x140dcd8)
#7 0x0001261ebfc0 in MessageLoop::RunInternal()+0x88 (XUL:arm64+0x1333fc0)
#8 0x0001261ebea0 in MessageLoop::Run()+0x60 (XUL:arm64+0x1333ea0)
#9 0x00012554c5b8 in nsThread::ThreadFunc(void*)+0x160 (XUL:arm64+0x6945b8)
#10 0x000105d3d558 in _pt_root+0x11c (libnss3.dylib:arm64+0x1c1558)
#11 0x000189149f90 in _pthread_start+0x84 (libsystem_pthread.dylib:arm64+0x6f90)
#12 0x718000189144d30 (<unknown module>)

Not the original one. After fixing it in the local, I can not reproduce the crash stack in the comment 0

Tyson, could you help me to double-check if you can reproduce the original stack?

Comment 6

[Eden Chuang[:edenchuang]]

Attachment: Bug 1790526 - Fix the nullptr assignment on CheckedUnsafePtr when Ubsan is enabled. r=#dom-worker-reviewers

Comment 7

[Tyson Smith [:tsmith]]

I am able to reproduce the issue with the patch applied.

A Pernosco session (with patch applied) is available here: https://pernos.co/debug/AKTDVC62g_oHMo8GmtoA1g/index.html

Comment 8

[Phabricator Automation]

Comment on attachment 9462487 [details]
Bug 1790526 - Fix the nullptr assignment on CheckedUnsafePtr when Ubsan is enabled. r=#dom-worker-reviewers

Revision D235952 was moved to bug 1947437. Setting attachment 9462487 [details] to obsolete.

Comment 9

[Eden Chuang[:edenchuang]]

According to the Pernosco trace, the propagated BrowsingContext has already been discarded. (Notice that the mID is not 0)
So GetMaybeDiscarded() could get nullptr at https://searchfox.org/mozilla-central/rev/7c573d9eb97e7b1ba383239bfac9260b26983544/toolkit/components/sessionstore/SessionStoreParent.cpp#210.

Comment 10

[Eden Chuang[:edenchuang]]

Attachment: Bug 1790526 - Handle the discarded BrowsingContext that propagated to SessionStoreParent. r=smaug

According to the Pernosco trace, https://static.pernos.co/server/7c640bc4ed587e824fab0ca8aa44f41d1b5595b5/rebuilding.html?redirect=https%3A%2F%2Fpernos.co%2Fdebug%2FAKTDVC62g_oHMo8GmtoA1g%2Findex.html, this is a case that the propagated BrowingContext has been discarded already, so it causes aBrowsingContext.GetMaybeDiscarded() get a nullptr.

This patch just check the propageted BrowsingContext's existence then to continue the IPC functions.

Comment 11

[Sam James]

Attachment: backtrace

Hi! I've spent the last few days debugging a crash when building w/ GCC and LTO and it appears to, in the end, be the same as this (https://bugs.gentoo.org/950229).



Testing the patch from https://bugzilla.mozilla.org/show_bug.cgi?id=1790526 (https://bugzilla.mozilla.org/attachment.cgi?id=9465385), but if it works, I think it'd make sense too.

https://searchfox.org/mozilla-central/source/toolkit/components/sessionstore/SessionStoreParent.cpp#197

because of the access of aBrowsingContext in:
         aBrowsingContext.GetMaybeDiscarded()->Canonical(), aFormData,
it might, with LTO, conclude "ok, it's not null, as you accessed it before", and then kill the mRawPtr check later (which would be legitimate)

Comment 12

[Sam James]

Sorry, I hit enter prematurely. Ignore the above comment.

Hi! I've spent the last few days debugging a crash when building w/ GCC and LTO and it appears to, in the end, be the same as this (https://bugs.gentoo.org/950229).

Testing the patch from https://bugzilla.mozilla.org/show_bug.cgi?id=1790526 (https://bugzilla.mozilla.org/attachment.cgi?id=9465385), appears to work, and I think it makes sense too.

Looking at https://searchfox.org/mozilla-central/source/toolkit/components/sessionstore/SessionStoreParent.cpp#197, because of the access of aBrowsingContext in:

         aBrowsingContext.GetMaybeDiscarded()->Canonical(), aFormData,

it might, with LTO, conclude "ok, it's not null, as you accessed it before", and then kill the mRawPtr check later (which would be legitimate)

In any case, the patch seems to fix the problem. Thanks!

(See backtrace at https://bugzilla.mozilla.org/attachment.cgi?id=9469376.)

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bd801b491ec6
Check if the propagated BrowsingContext is discarded or not in SessionStoreParent. r=smaug

Comment 14

[Serban Stanca [:SerbanS]]

Backed out for causing mochitests failures in browser_formdata.js.

• Backout link
• Push with failures
• Failure Log
• Failure line: TEST-UNEXPECTED-FAIL | browser/components/sessionstore/test/browser_formdata.js | Uncaught exception in test bound test_formdata - at chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_formdata.js:47 - TypeError: can't access property "txt", formdata.id is undefined

Please also check these mochitests failures.

• Failure Log

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4159736c2897
Handle the discarded BrowsingContext that propagated to SessionStoreParent. r=smaug

Comment 16

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/4159736c2897