1474172 - Crash in _PR_NativeRunThread | static unsigned int pr_root

Status: RESOLVED INCOMPLETE

(Core :: General, defect, P2)

Description

[[:philipp]]

This bug was filed from the Socorro interface and is
report bp-93b07f75-c127-4e7e-ac33-990990180707.
=============================================================

Top 8 frames of crashing thread:

0  @0xbaadbaad 
1 nss3.dll _PR_NativeRunThread nsprpub/pr/src/threads/combined/pruthr.c:397
2 nss3.dll static unsigned int pr_root nsprpub/pr/src/md/windows/w95thred.c:137
3 ucrtbase.dll thread_start<unsigned int > 
4 kernel32.dll BaseThreadInitThunk 
5 mozglue.dll static void patched_BaseThreadInitThunk mozglue/build/WindowsDllBlocklist.cpp:622
6 ntdll.dll __RtlUserThreadStart 
7 ntdll.dll _RtlUserThreadStart 

=============================================================

these crash signatures are spiking up in volume during the 62 cycle on windows. a number of the are looking like wildptr-issues.

Comment 1

[Daniel Veditz [:dveditz]]

Almost all of these are EXCEPTION_ACCESS_VIOLATION_EXEC which is highly exploitable if you can control data. But hard to tell how it got corrupted or even what team should be looking into this. The only place I see 0xbaadbaad in our codebase is in cubeb, but that's only in one crash this week (just happened to be the one in comment 0). 0xcd09b400 shows up nine times but couldn't find it in our codebase. 0x00630069 shows up nine times and /might/ be text ("ci") but that's not enough to go on. Maybe there's more in a mini-dump? 60% of these say they're in the content process, the rest are blank (does that mean parent?).

Paul: anything change in audio threading in 62?

Comment 2

[Paul Adenot (:padenot)]

The file the 0xbaadbaad is in is only compiled in Android builds, and only ever executed in very old android version, in exceptional circumstances (as a last-resort fallback).

It must be something else.

Comment 3

[Daniel Veditz [:dveditz]]

Since we don't know who ought to be looking into this bug core-security-release gives it the most visibility within trusted security-bug fixers.

All the recent crashes I see here are Win 7 in case that's a clue.

We have so little information maybe it's better to treat this as a public topcrash rather than a security bug. It's definitely going to get worse once Fx62 ships.

Comment 4

[Liz Henry (:lizzard)]

I don't see any emails in the crash reports. 
URLs are mostly YouTube and then various adult sites. 

Randell do you have any ideas here?

Comment 5

[Randell Jesup [:jesup] (needinfo me)]

We have a few crashes that look similar in 61 and 61b13/14.  Was there a compiler change around then, maybe?  (probably not)
The 61b3 crashes are nullptr writes, different.  Then it spiked in 62

The 1 windows 10 crash was in DNS Resolver; might be different, maybe, or it might be a clue.

The others are high thread #'s mostly, implying dynamic threads, but not all (thread 26 for example)

All are in whatever's below _PR_NativeRunThread directly (but they have to be to have this signature).

Trashing stack?

AV?

(100.0% in signature vs 53.71% overall) Module "samlib.dll" = true
(100.0% in signature vs 56.14% overall) Module "WSHTCPIP.DLL" = true

Comment 6

[Randell Jesup [:jesup] (needinfo me)]

Any thoughts?

Comment 7

[Nathan Froyd [:froydnj]]

No good ideas offhand, other than loading some of these minidumps in windbg, and either seeing if windbg offers up a better stacktrace, or manually unwinding the stack to see what's going on.

We switched to MSVC 15.6 in bug 1424281 (ff61) and updated to 15.6.6 in bug 1453317 (ff61).  The last update before that was bug 1408789, where we went to 15.4.1 (ff58).  So maybe the 15.6.x updates are causing problems?

Comment 8

[Liz Henry (:lizzard)]

David, is this something you might have time to investigate? Or, can you suggest someone?

Comment 9

[(Away)]

Someone with debugger access should check !analyze -v for Last Control Transfer to see where the jump came from. That's all I have.

Comment 10

[Nathan Froyd [:froydnj]]

OK, I looked at minidumps and I still don't understand what's going wrong.  According to windbg, the "real" stack for the crash in comment 0 looks like:

 # ChildEBP RetAddr  
WARNING: Frame IP not in any known module. Following frames may be wrong.
00 184afcf8 510ea313 0xbaadbaad
01 184afd04 60e424f8 xul!ReleaseObject+0xc
02 184afd24 60e37820 nss3!_PR_NativeRunThread+0xcc
03 184afd30 60f3e89f nss3!pr_root+0xd
04 184afd6c 76b21174 ucrtbase!thread_start<unsigned int (__stdcall*)(void *)>+0x3f
05 184afd78 617efb2d kernel32!BaseThreadInitThunk+0xe
06 184afd94 7729b3f5 mozglue!patched_BaseThreadInitThunk+0x3d
07 184afdd4 7729b3c8 ntdll!__RtlUserThreadStart+0x70

I looked at the dump of stack memory just to be sure, and this stack seems correct.

xul!ReleaseObject is this bit of code from nsThreadManager.cpp:

static void
ReleaseObject(void* aData)
{
  static_cast<nsISupports*>(aData)->Release();
}

ReleaseObject is used as the "destructor" for the thread-specific value of the current thread, set up when nsThreadManager initializes.

  if (PR_NewThreadPrivateIndex(&mCurThreadIndex, ReleaseObject) == PR_FAILURE) {
    return NS_ERROR_FAILURE;
  }

The value held in "TLS" here is a strong reference to the nsThread structure itself.

The only place that the destructor would get called are:

1) When we unregister various threads from the thread manager; and
2) When we unregister the main thread during shutdown.

These crashes do not happen on the main thread, so presumably we're not in case #2.  That means we're in case #1.  But if we were in case #1, I'd expect a lot more stuff to be on the stack, since we're actually inside the nsThread's thread function:

https://dxr.mozilla.org/mozilla-beta/source/xpcom/threads/nsThread.cpp#381-475

when we set the "current thread" thread-specific value to nullptr, via the nsThreadManager::UnregisterThread call.  We are also still holding *another* strong reference to the nsThread itself when we set said value to nullptr, so that Release() call, above, *should* still have a valid value to operate on.  Clearly it does not.  But I don't know how we would be calling ReleaseObject without more stuff on the stack.

And other minidumps (OK, one) don't even have ReleaseObject on the stack, they have some weird Windows-internal audio calls getting called directly from _PR_NativeRunThread. (AudioSes!ATL::CComObject<CAudioSessionManagerClient>::`vftable' or so if that means anything.)

So, flailing wildly here, but to repeat Dan's question from comment 1, did anything audio+Windows related change in 62?  We might be looking at memory corruption leading to jumping to bad places from vtables for all I know.

Comment 11

[Paul Adenot (:padenot)]

There is no code change in libcubeb for windows code between release and beta. However, I seem to remember some movement on device change notifications in bug 1449388, that is windows specific and audio related. I don't have access to this bug so I can't say much.

cc-ing kinetik, I think he knows more than me on the matter iirc.

Again, 0xbaadbaad is compiled only on Android, so this might be something completely unrelated to audio.

Comment 12

[Randell Jesup [:jesup] (needinfo me)]

Though some bit of windows or non-mozilla code might use baadbaad, perhaps....

Comment 13

[Matthew Gregan [:kinetik]]

(In reply to Paul Adenot (:padenot) from comment #11)
> There is no code change in libcubeb for windows code between release and
> beta. However, I seem to remember some movement on device change
> notifications in bug 1449388, that is windows specific and audio related. I
> don't have access to this bug so I can't say much.

Bug 1449388 was Flash related - our sandboxing broke Flash's (second, working around earlier sandboxing restrictions) way of registering for audio device notifications.  The fix was uplifted to 62b7, so it's in the range for this issue.  I'll cc David on this bug, since he worked on bug 1449388 and might have ideas.

We've had other mystery CAudioSession-related crashes on Windows 7 in the past too.  Bug 1427011 (landed in 61) moved the notification registration out of the content process for libcubeb (so we only register in the chrome process and rely on Gecko's remoted AudioSession in the content process) to try to narrow down the responsible code, with ambiguous results.

> cc-ing kinetik, I think he knows more than me on the matter iirc.
> 
> Again, 0xbaadbaad is compiled only on Android, so this might be something
> completely unrelated to audio.

Right, although the AudioSes!ATL::CComObject<CAudioSessionManagerClient> reference in comment 10 is suspicious and potentially has some relation to the changes in changes bug 1449388.

Note that the threads inside libcubeb are raw Windows threads (via _beginthreadex) and don't use Gecko's thread management, so if the PR_NativeRunThread part of the stack is valid, these are not libcubeb threads.

Comment 14

[Daniel Veditz [:dveditz]]

These crash signatures all stop abruptly after August 26. Amazing they last all the way to the last beta (62.0b20) and then stop a week before we ship 62.0 release builds. We probably gave the release candidate to beta channel people around then but that's not much of an explanation. Something that got fixed in Firefox 63, but also happened to be in code not built in Release?

Unless we see any more of these maybe it's time for WORKSFORME

Comment 15

[[:philipp]]

unfortunately the signature just seems to have shifted.

Comment 16

[Randell Jesup [:jesup] (needinfo me)]

FYI, for https://crash-stats.mozilla.com/report/index/5fe8e2af-9960-409d-8ce0-3cee60180910, it appears that the crash shown in windbg (which doesn't match the crashreport, note) is below google_breakpad::ExceptionHandler::WriteMinidumpOnHandlerThread() apparently in WaitForSingleObject(handler_finish_semaphore_,INFINITE) -- and the wndbg stack shows it in WaitForSingleObject is the next frame down on the stack, and the final location on the stack is ZwWaitForSingleObject + 0xa.

I'm no windbg expert, though note.  That stack frame was from ~55s which both windbg and the crashreport agree is the crashing thread.  Nathan?

Who's handling breakpad now?

Comment 17

[Nathan Froyd [:froydnj]]

(In reply to Randell Jesup [:jesup] from comment #16)
> FYI, for
> https://crash-stats.mozilla.com/report/index/5fe8e2af-9960-409d-8ce0-
> 3cee60180910, it appears that the crash shown in windbg (which doesn't match
> the crashreport, note) is below
> google_breakpad::ExceptionHandler::WriteMinidumpOnHandlerThread() apparently
> in WaitForSingleObject(handler_finish_semaphore_,INFINITE) -- and the wndbg
> stack shows it in WaitForSingleObject is the next frame down on the stack,
> and the final location on the stack is ZwWaitForSingleObject + 0xa.
> 
> I'm no windbg expert, though note.  That stack frame was from ~55s which
> both windbg and the crashreport agree is the crashing thread.  Nathan?

I saw the same when I opened things up in windbg, but I assumed that was just an artifact of the crash reporting process, rather than where things were going wrong.  I'm not sure what the question is here.

Comment 18

[(not currently active) Ted Mielczarek]

Note that you always need to explicitly specify that WinDBG should use the exception context with the `.ecxr` command when loading a minidump. Otherwise you get the stack from when the dump was being written, which naturally goes through some Breakpad code.

Comment 19

[(not currently active) Ted Mielczarek]

(In reply to David Major [:dmajor] from comment #9)
> Someone with debugger access should check !analyze -v for Last Control
> Transfer to see where the jump came from. That's all I have.

FWIW, for https://crash-stats.mozilla.com/report/index/5fe8e2af-9960-409d-8ce0-3cee60180910 this shows:

LAST_CONTROL_TRANSFER:  from 000007fef305ab6a to 0000000007fefaee

STACK_TEXT:  
00000000`1605fcf8 000007fe`f305ab6a : 00000000`00825c00 00000000`00000000 00000000`00000000 00000000`00000000 : 0x7fefaee
00000000`1605fd00 000007fe`f304dcf6 : 00000000`0bbbe2e0 00000000`1a1200f0 00000000`00000030 00000000`1605fd78 : nss3!_PR_NativeRunThread+0xee
00000000`1605fd30 000007fe`f35c0399 : 00000000`00000000 00000000`1a1200f0 00000000`00000000 00000000`00000030 : nss3!pr_root+0xa
00000000`1605fd60 00000000`0002012b : 00000000`00000000 00000000`0008d000 00000020`00001000 00000000`01000000 : ucrtbase!thread_start<unsigned int (__cdecl*)(void * __ptr64)>+0x39
00000000`1605fd90 00000000`00000000 : 00000000`0008d000 00000020`00001000 00000000`01000000 00000000`00000000 : 0x2012b

Also the address it jumped to seems to just be free memory:
0:055> !address 7fefaee

                                     
Mapping file section regions...
Mapping module regions...
Mapping PEB regions...
Mapping TEB and stack regions...
Mapping heap regions...
Mapping page heap regions...
Mapping other regions...
Mapping stack trace database regions...
Mapping activation context regions...

Usage:                  Free
Base Address:           00000000`03d41000
End Address:            00000000`08320000
Region Size:            00000000`045df000 (  69.871 MB)
State:                  00010000          MEM_FREE
Protect:                00000001          PAGE_NOACCESS
Type:                   <info not present at the target>


Content source: 0 (invalid), length: 62e8ca

Comment 20

[(not currently active) Ted Mielczarek]

One thing I noticed looking at the signature queries here is that all of the reports here seem to be on Windows 7.

Comment 21

[Randell Jesup [:jesup] (needinfo me)]

Odd that the crash address almost looks like a code address:
    0x7fef305ab6a ->
0x00007fefaee

Almost like the address was right-shifted by 16 bits (or an address used to load a code pointer (vtbl?) was 2 bytes off....)  Perhaps it's just chance

Comment 22

[Randell Jesup [:jesup] (needinfo me)]

And conversely, jumping into free memory would explain the exec crashes; the question would be why it's freed.  The Win7 aspect is interesting, if this has something to do with some system code.

the missing-lowest-16-bits (apparent) aspect seems to show up in a lot of the 62 crashes -- but not quite all.  Interestingly, the pattern seems different between 62.0bN and 62.0:

https://crash-stats.mozilla.com/signature/?version=62.0&version=62.0b&signature=PR_NativeRunThread%20%7C%20pr_root&date=%3E%3D2018-08-11T11%3A33%3A36.000Z&date=%3C2018-09-11T11%3A33%3A36.000Z&_columns=date&_columns=product&_columns=version&_columns=build_id&_columns=platform&_columns=reason&_columns=address&_columns=install_time&_sort=-date&page=1#reports

Comment 23

[Andrew Overholt [:overholt]]

This doesn't seem actionable but given it's sec-high I'll mark it as P2. Is it worth someone on your team taking another stab at a fix here, Eric?

Comment 24

[Eric Rahm [:erahm]]

Nathan, any chance you can look at a couple more recent dumps in windbg to see if anything sticks out? This code has morphed a bit over the past few months.

Comment 25

[Liz Henry (:lizzard)]

Following up the n-i in email to see if this can get one more look before declaring it stalled.

Comment 26

[Nathan Froyd [:froydnj]]

I looked at a few minidumps, including !analyze -v, which didn't turn up anything of interest. Comment 20 noted that all of these crashes occur on Windows 7; it looks like for at least one signature:

https://crash-stats.mozilla.org/signature/?signature=%400x0%20%7C%20_PR_NativeRunThread%20%7C%20pr_root&date=%3E%3D2019-08-29T14%3A33%3A00.000Z&date=%3C2019-09-05T14%3A33%3A00.000Z&_columns=date&_columns=product&_columns=version&_columns=build_id&_columns=platform&_columns=reason&_columns=address&_columns=platform_version&_sort=-date&page=1

all the crashes seem to be happening on 6.1.7600 or 6.1.7601 SP 1. (Are there newer patches/service packs for Win7 that might have fixed this bug?)

The crashes for a slightly different signature also appear to be on all the same Windows 7 versions as above:

https://crash-stats.mozilla.org/signature/?signature=_PR_NativeRunThread%20%7C%20pr_root&date=%3E%3D2019-08-29T15%3A14%3A00.000Z&date=%3C2019-09-05T15%3A14%3A00.000Z&_columns=date&_columns=product&_columns=version&_columns=build_id&_columns=platform&_columns=reason&_columns=address&_columns=platform_version&_sort=-date&page=1

CPU versions appear to be all over the place, so I don't think we can implicate the CPU.

It's not super common, but when the crashing thread has a name--maybe a quarter of the crashes or so--it's "AudioASCReleaser", which points at bug 1419488. That bug was intended to fix a crash specific to Windows 7, which, lo and behold, is also the only version implicated here. Unfortunately, I can't look at the crash reports for that bug, because they're much older than 6 months. I would be interested to know exactly what Windows 7 versions the crashes from that bug were happening with.

I am going to go out on a limb here and say that this is some Windows version-specific bug that we can't do a lot with.

Comment 27

[David Lawrence [:dkl]]

Removing employee no longer with company from CC list of private bugs.

Comment 28

[Tom Ritter [:tjr]]

6 of the signatures are dead. The remaining 3 implicate Windows 7 disproportionately, but one has a reasonable number (35%) on Windows 10. Two of the remaining three signatures only occur on ESR 102/115 or sub-100 builds. The remaining signature is the interesting one: _PR_NativeRunThread | pr_root

It gets ~ 1 crash per major release, there's 24 on a version over 113. There's some breakpoint reasons, all but one related to shutdown hangs. They all have mozilla::interceptor::FuncHook<mozilla::interceptor::WindowsDllInterceptor<mozilla::interceptor::VMSharingPolicyShared>, void (*)(int, void*, void*)>::operator()(int&, void*&, void*&) const in the stack. The algorithm shows no real probability of bit flips.

Closing this as unactionable.

Comment 29

[BugBot (nomail) [:suhaib / :marco/ :calixte]]

Since the bug is closed, the stalled keyword is now meaningless.
For more information, please visit BugBot documentation.