Open Bug 1057551 Opened 6 years ago Updated 2 years ago

NULL deref in MOZ_RELEASE_ASSERT with ASAN detect_stack_use_after_return

Categories

(Core :: MFBT, defect, critical)

x86_64
Linux
defect
Not set
critical

Tracking

()

Tracking Status
firefox60 --- affected

People

(Reporter: truber, Unassigned)

References

(Blocks 1 open bug)

Details

(Keywords: crash)

Attachments

(1 file)

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:31.0) Gecko/20100101 Firefox/31.0 (Beta/Release)
Build ID: 20140715214327

Steps to reproduce:

Ran Firefox nightly from https://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-inbound-linux64-asan/1408681878/ (20140821213118) with ASAN_OPTIONS=detect_stack_use_after_return=1


Actual results:

Firefox crashed during startup with the following ASAN log:
=================================================================
==15897==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7f7d5c2e02f4 sp 0x7fff9806c600 bp 0x7fff9806c610 T0)
    #0 0x7f7d5c2e02f3 (libxul.so!XPCJSContextStack::InitSafeJSContext()+0x113)
	Line 126 of "/builds/slave/m-in-l64-asan-0000000000000000/build/js/xpconnect/src/XPCJSContextStack.cpp"
    #1 0x7f7d5c396dea (libxul.so!nsXPConnect::InitStatics()+0x30a)
	Line 144 of "/builds/slave/m-in-l64-asan-0000000000000000/build/js/xpconnect/src/nsXPConnect.cpp"
    #2 0x7f7d5c324418 (libxul.so!xpcModuleCtor()+0x8)
	Line 13 of "/builds/slave/m-in-l64-asan-0000000000000000/build/js/xpconnect/src/XPCModule.cpp"
    #3 0x7f7d6074dcf4 (libxul.so!Initialize()+0x44)
	Line 382 of "/builds/slave/m-in-l64-asan-0000000000000000/build/layout/build/nsLayoutModule.cpp"
    #4 0x7f7d5b3429ad (libxul.so!nsFactoryEntry::GetFactory()+0x24d)
	Line 849 of "/builds/slave/m-in-l64-asan-0000000000000000/build/xpcom/components/nsComponentManager.cpp"
    #5 0x7f7d5b3439bf (libxul.so!nsComponentManagerImpl::CreateInstanceByContractID(char const*, nsISupports*, nsID const&, void**)+0x13f)
	Line 1187 of "/builds/slave/m-in-l64-asan-0000000000000000/build/xpcom/components/nsComponentManager.cpp"
    #6 0x7f7d5b33abc1 (libxul.so!nsComponentManagerImpl::GetServiceByContractID(char const*, nsID const&, void**)+0xa51)
	Line 1551 of "/builds/slave/m-in-l64-asan-0000000000000000/build/xpcom/components/nsComponentManager.cpp"
    #7 0x7f7d5b3a9867 (libxul.so!nsCOMPtr_base::assign_from_gs_contractid(nsGetServiceByContractID, nsID const&)+0xb7)
	Line 67 of "/builds/slave/m-in-l64-asan-0000000000000000/build/xpcom/glue/nsComponentManagerUtils.cpp"
    #8 0x7f7d5b3d3bfb (libxul.so!NS_InitXPCOM2+0xd2b)
	Line 969 of "/builds/slave/m-in-l64-asan-0000000000000000/build/xpcom/build/../glue/nsCOMPtr.h"
    #9 0x7f7d61341bcc (libxul.so!XREMain::XRE_main(int, char**, nsXREAppData const*)+0x46c)
	Line 1265 of "/builds/slave/m-in-l64-asan-0000000000000000/build/toolkit/xre/nsAppRunner.cpp"
    #10 0x7f7d61342acd (libxul.so!XRE_main+0x3dd)
	Line 4309 of "/builds/slave/m-in-l64-asan-0000000000000000/build/toolkit/xre/nsAppRunner.cpp"
    #11 0x48a2f7 (firefox!main+0xa67)
	Line 282 of "/builds/slave/m-in-l64-asan-0000000000000000/build/browser/app/nsBrowserApp.cpp"
    #12 0x7f7d6af3eec4 (libc.so.6!__libc_start_main+0xf4)
	Line 287 of "libc-start.c"
    #13 0x48975c (firefox!_start+0x28)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV ??:0 ??
==15897==ABORTING


Expected results:

Firefox should run normally unless stack use-after-return is detected.
Product: Firefox → Core
Component: Untriaged → XPConnect
Status: UNCONFIRMED → NEW
Ever confirmed: true
Attached file log.txt
Still reproduces with original instructions. Attaching updated log from mozilla-central rev b1d60f2f68c7
Severity: normal → critical
Keywords: crash
Tyson showed me a newer log. It looks like the crash is here:
  MOZ_RELEASE_ASSERT(gSystemPrincipal);

I have no idea why that's a SEGV. Maybe MOZ_RELEASE_ASSERT is making ASan angry?
MOZ_RELEASE_ASSERT will dereference NULL to portably crash the browser.  Is that what's happening here?
I asked Tyson to check what happens with MOZ_RELEASE_ASSERT(gSystemPrincipal). gSystemPrincipal is just a raw pointer so I wouldn't think null checking it could cause any issues.
I guess we're all looking at this at the same time, analysis for comment 1.

The latest stack from 2016-09-29:

>    #0 0x7fd449e940c3 in AddRef /home/worker/workspace/build/src/js/xpconnect/src/nsXPConnect.cpp:43:1
>    #1 0x7fd449e940c3 in nsXPConnect::InitStatics() /home/worker/workspace/build/src/js/xpconnect/src/nsXPConnect.cpp:117

That points to [1]:

>     NS_ADDREF(gSelf);

I have no idea why this is tripping up detect_stack_use_after_return [2], gSelf is a global. We could allocate into a RefPtr and then .forget into gSelf and see if that appeases it.

Or is the case it's just segfaulting here, but it's not actually tripping up detect_stack_use_after_return? Again that makes no sense unless there was possibly a thread that grabs gSelf in between allocation and addref, addrefs and releases and deletes it from under us. But in theory all of this code is main thread only

[1] https://hg.mozilla.org/mozilla-central/annotate/d72527b7d3c0/js/xpconnect/src/nsXPConnect.cpp#l117
[2] https://github.com/google/sanitizers/wiki/AddressSanitizerUseAfterReturn
(In reply to Nathan Froyd [:froydnj] from comment #3)
> MOZ_RELEASE_ASSERT will dereference NULL to portably crash the browser.  Is
> that what's happening here?

I wonder if this bit [1] is giving it a fit:

>        do {
>          *((volatile int*) NULL) = line;
>          ::abort();
>        } while (0)

It's hard for me to believe that's the first MOZ_RELEASE_ASSERT we hit.

[1] https://searchfox.org/mozilla-central/rev/bd05e3853c6e982e2a35c1cc404b987b2bc914d6/mfbt/Assertions.h#225-229
From mozilla-central changeset: 404814:5e9bd04333f2

==20243==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7ff4dc78861b bp 0x7ffcdf646aa0 sp 0x7ffcdf646a80 T0)
==20243==The signal is caused by a WRITE memory access.
==20243==Hint: address points to the zero page.
    #0 0x7ff4dc78861a in nsXPConnect::InitStatics() src/js/xpconnect/src/nsXPConnect.cpp:134:5
    #1 0x7ff4dc700c98 in xpcModuleCtor() src/js/xpconnect/src/XPCModule.cpp:13:5
    #2 0x7ff4e3d52c27 in Initialize() src/layout/build/nsLayoutModule.cpp:295:8
    #3 0x7ff4da4d81ec in Load src/xpcom/components/nsComponentManager.cpp:763:21
    #4 0x7ff4da4d81ec in nsFactoryEntry::GetFactory() src/xpcom/components/nsComponentManager.cpp:1785
    #5 0x7ff4da4d952d in nsComponentManagerImpl::CreateInstanceByContractID(char const*, nsISupports*, nsID const&, void**) src/xpcom/components/nsComponentManager.cpp:1083:41
    #6 0x7ff4da4d0536 in nsComponentManagerImpl::GetServiceByContractID(char const*, nsID const&, void**) src/xpcom/components/nsComponentManager.cpp:1446:10
    #7 0x7ff4da4e00ac in CallGetService src/xpcom/components/nsComponentManagerUtils.cpp:67:43
    #8 0x7ff4da4e00ac in nsGetServiceByContractID::operator()(nsID const&, void**) const src/xpcom/components/nsComponentManagerUtils.cpp:280
    #9 0x7ff4da3823de in nsCOMPtr_base::assign_from_gs_contractid(nsGetServiceByContractID, nsID const&) src/xpcom/base/nsCOMPtr.cpp:95:7
    #10 0x7ff4da590e48 in nsCOMPtr src/objdir-ff-asan/dist/include/nsCOMPtr.h:928:5
    #11 0x7ff4da590e48 in NS_InitXPCOM2 src/xpcom/build/XPCOMInit.cpp:703
    #12 0x7ff4e7978668 in Initialize src/toolkit/xre/nsAppRunner.cpp:1569:8
    #13 0x7ff4e7978668 in XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) src/toolkit/xre/nsAppRunner.cpp:4804
    #14 0x7ff4e7979ac5 in XRE_main(int, char**, mozilla::BootstrapConfig const&) src/toolkit/xre/nsAppRunner.cpp:4900:21
    #15 0x51657b in do_main src/browser/app/nsBrowserApp.cpp:231:22
    #16 0x51657b in main src/browser/app/nsBrowserApp.cpp:304
    #17 0x7ff4fb9ed1c0 in __libc_start_main /build/glibc-itYbWN/glibc-2.26/csu/../csu/libc-start.c:308
    #18 0x41edf9 in _start (src/objdir-ff-asan/dist/bin/firefox+0x41edf9)


I still see this with mccr8's suggestion applied:

--- a/js/xpconnect/src/nsXPConnect.cpp  Thu Feb 22 17:28:59 2018 +0200
+++ b/js/xpconnect/src/nsXPConnect.cpp  Thu Feb 22 14:14:31 2018 -0800
@@ -131,7 +131,7 @@
     nsScriptSecurityManager::InitStatics();
     gScriptSecurityManager = nsScriptSecurityManager::GetScriptSecurityManager();
     gScriptSecurityManager->GetSystemPrincipal(&gSystemPrincipal);
-    MOZ_RELEASE_ASSERT(gSystemPrincipal);
+    MOZ_RELEASE_ASSERT(true);
With a debug build I see:

Hit MOZ_CRASH(InitSelfHostedCode failed) at /builds/worker/workspace/build/src/js/xpconnect/src/nsXPConnect.cpp:138
Component: XPConnect → MFBT
Summary: NULL deref in nightly with ASAN detect_stack_use_after_return → NULL deref in MOZ_RELEASE_ASSERT with ASAN detect_stack_use_after_return
You need to log in before you can comment on or make changes to this bug.