firefox crashed with SIGSEGV in NSSRWLock_LockRead_Util()

[Thomas N]

Binary package hint: firefox-3.0

Don't really know what I did but it obviously crashed...

ProblemType: Crash
Architecture: i386
Date: Mon Jun 2 19:16:26 2008
DistroRelease: Ubuntu 8.04
ExecutablePath: /usr/lib/firefox-3.0/firefox
Package: firefox-3.0 3.0~rc1+nobinonly-0ubuntu0.8.04.1
PackageArchitecture: i386
ProcCmdline: /usr/lib/firefox-3.0/firefox -height 100 -width 100
ProcEnviron:
 PATH=/home/username/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games
 LANG=en_DK.UTF-8
 SHELL=/bin/bash
Signal: 11
SourcePackage: firefox-3.0
StacktraceTop:
 NSSRWLock_LockRead_Util () from /usr/lib/libnssutil3.so.1d
 SECMOD_GetReadLock () from /usr/lib/libnss3.so.1d
 PK11_GetAllTokens () from /usr/lib/libnss3.so.1d
 PK11_GetBestSlotMultiple () from /usr/lib/libnss3.so.1d
 PK11_GetBestSlot () from /usr/lib/libnss3.so.1d
Title: firefox crashed with SIGSEGV in NSSRWLock_LockRead_Util()
Uname: Linux 2.6.24-17-generic i686
UserGroups: adm admin audio cdrom dialout dip floppy fuse lpadmin plugdev sambashare vboxusers video

[Timeless-bemail (timeless-bemail)]

oh right, from my naive perspective, this is a topcrash.
http://crash-stats.mozilla.com/report/list?range_unit=weeks&version=Firefox%3A3.0pre&range_value=2&signature=NSSRWLock_LockRead_Util

[Nelson-bolyard (nelson-bolyard)]

Explain how it is that this process is not initializing NSS until after it
has run so long that it is already in an out-of-memory condition.

[Kai Engert (kaie)]

Nelson, Mozilla software will not access PSM unless it has a need to. And only when PSM gets accessed the first time it will init NSS.

[Kai Engert (kaie)]

According to my understanding, it seems reasonable to start by getting a lock, and only afterwards try to get the module list.

But I don't know! I don't know this code well.

Looking at http://lxr.mozilla.org/seamonkey/ident?i=SECMOD_GetDefaultModuleList
it appears that usually the module list is obtained first, and only afterwards the lock is obtained and locked.

I suspect that the lock is only required for modifying the contents of the lock?

[Timeless-bemail (timeless-bemail)]

The crash report comment quoted here and the reports i read show this probably *is* early in startup.

I think we use the hashing bits as part of the update service and it seems likely it'd be nearly the first crypto bit to be hit.

please note that i use systems where memory is often but not always scarce. It isn't legal to assume early allocs will succeed any more than it is to assume late ones will.

is there a public contract for "HASH_Create" somewhere?

[Rrelyea (rrelyea)]

Ouch...

The NSS contract is NO nss calls are made until NSS is initialized (period). We guarrentee you a crash if you do.

To date, PSM has been the only user of NSS inside a mozilla app.

If someone outside PSM is calling NSS directly, they, at the very least, need to do the minimal xpcom calls to access the crypto object (exported by PSM). Simply setting up that object will guarrentee NSS is initialized properly.

That does not necessarily mean you are home free (PSM keeps track of the NSS objects it creates to make sure it closes them all down if mozilla wants to switch users, for instance, as well as keeps track that mozilla isn't in the middle of shutting down (to make sure no calls are made after NSS is shutdown).

bob

> I suspect that the lock is only required for modifying the contents of the
> lock?

No, the lock is required for both reading and writing. Looking at the code, it's clear it should be acquired before getting the list (as adding, and removal could in theory change the pointer).

In practice the first module is the softoken, which is never removed (though it may change character), so such a bug is unlikely to show up. (Actually changes to the module list is extremely rarer anyway. Adding and deleting pkcs #11 modules in mozilla are the only way it happens in any real application that I know of.

[Timeless-bemail (timeless-bemail)]

right, I claim that the patch I'm providing here is correct based on your comments. And I'd like it to be taken.

I think that PSM itself may be violating the NSS initialization constraint, but this bug is about flaws in the NSS code. I'll deal w/ any PSM errors in another bug.

Kaie: i think comment 6 should be sufficient for you to r+ attachment 314290

[Cjcypoi02 (cjcypoi02)]

See Bug 410897 Comment 7 (and 8)

[Thomas N (konstigt-deactivatedaccount)]

Binary package hint: firefox-3.0

Don't really know what I did but it obviously crashed...

ProblemType: Crash
Architecture: i386
Date: Mon Jun 2 19:16:26 2008
DistroRelease: Ubuntu 8.04
ExecutablePath: /usr/lib/firefox-3.0/firefox
Package: firefox-3.0 3.0~rc1+nobinonly-0ubuntu0.8.04.1
PackageArchitecture: i386
ProcCmdline: /usr/lib/firefox-3.0/firefox -height 100 -width 100
ProcEnviron:
 PATH=/home/username/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games
 LANG=en_DK.UTF-8
 SHELL=/bin/bash
Signal: 11
SourcePackage: firefox-3.0
StacktraceTop:
 NSSRWLock_LockRead_Util () from /usr/lib/libnssutil3.so.1d
 SECMOD_GetReadLock () from /usr/lib/libnss3.so.1d
 PK11_GetAllTokens () from /usr/lib/libnss3.so.1d
 PK11_GetBestSlotMultiple () from /usr/lib/libnss3.so.1d
 PK11_GetBestSlot () from /usr/lib/libnss3.so.1d
Title: firefox crashed with SIGSEGV in NSSRWLock_LockRead_Util()
Uname: Linux 2.6.24-17-generic i686
UserGroups: adm admin audio cdrom dialout dip floppy fuse lpadmin plugdev sambashare vboxusers video

[David Weitz (dabbaking)]

Just got this crash: http://crash-stats.mozilla.com/report/index/7db16dc5-3100-11dd-9e8b-0013211cbf8a?p=1

[Nelson-bolyard (nelson-bolyard)]

Bob's comment 6 should be interpreted as saying this:

Crashing is the DEFINED BEHAVIOR of NSS when NSS functions are called
before NSS is initialized. It is the caller's responsibility to ensure
that NSS is initialized before calling other NSS functions (besides
NSS initialization functions).

So, by definition, this is NOT an NSS bug.
TWO NSS module owners have now spoken on this subject.

[Samuel-sidler+old (samuel-sidler+old)]

Nelson, where should we move this bug to to get the needed attention (and an acceptable patch)?

[Nelson-bolyard (nelson-bolyard)]

Samuel: I think I just moved it to the right place when I added comment 10.

[Samuel-sidler+old (samuel-sidler+old)]

Oh, sorry. I missed that in the Bug Activity. Thanks!

[Dveditz (dveditz)]

I think we've got the same topcrash on branch, would be nice to fix this there as well if the patch gets reviews and such.

[Kai Engert (kaie)]

nsNSSModule.cpp has its own copied+modified XPCOM constructors.
Those are designed to do the following:
- if the interface requested means we create an instance of
  class nsNSSComponent, then proceed (because that class
  implements the code that is needed to init NSS)
- if the above is not true, then we first jump through function
  EnsureNSSInitialized and if necessary create the nsNSSComponent singleton.

The crashing code is in nsCryptoHash, which uses the correct macro that tries to achieve the latter.

If this bug is because NSS was not initialized, it means either
- the EnsureNSSInitialized code does not work (or regressed)
or
- you have shut NSS. You might have sent out a profile changed notification, but not yet switched to a new profile

The existing PSM code assumes that you'll never use any of PSM services while you are in the middle of a profile switch.

[Kai Engert (kaie)]

We should trace the calls to
  NSS_InitReadWrite
  NSS_Shutdown
  HASH_Create

If we see calls in the above order, then we've found the cause of the bug.

We must find a way to ensure it never happens.

[Kai Engert (kaie)]

Oops. We missed something when nsICryptoHash got implemented in bug 415799.

=> nsNSSShutDownObject

nsCryptoHash and nsCryptoHMAC contain a handle to an NSS object that gets invalid at time of NSS shutdown.

We should have made these classes derive from nsNSSShutDownObject, in order to track/invalidate the references when necessary.

[Honzab-moz (honzab-moz)]

timeless: it is really pity you didn't get the JS stack when it crashed. Do you know about a way to reproduce it?

The theory with the update service seems to me right. I experienced some crashes while closing firefox and its autoupdate was in progress (there were active download).

[Honzab-moz (honzab-moz)]

Just for report: I created fake update file on my local server and I emulate the auto update being in progress in background. When I shutdown Minefield it does NOT crash - tried 10 times...

The theory with update service will probably be wrong because: nsUpdateService.js!_verifyDownload that creates cryptohash instance is called only from OnStopRequest with status NS_OK. When shutdown with download in progress, the download request is canceled from "xpcom-shutdown" notification with NS_ERROR_ABORT status - so _verifyDownload is not called. Anyway, NSS is shutdown by nsNSSComponent in "profile-before-change" event that comes earlier then "xpcom-shutdown". If the download would succeed between these two events and OnStopRequest would be passed to JS somehow we theoretically may crash. But it is highly unlikely to happen.

[Timeless-bemail (timeless-bemail)]

this was a crash report based bug. i didn't experience it. i saw it in reports and picked a series of paths which aren't correct to patch. they're still not correct. fwiw, it's easy to tell if a crash isn't mine. I only run XP, so if the crash report i pick is vista (6.*) then it can't be mine :).

that psm is broken is quite likely (mentioned in comment 0).

do note that the crash report i picked has a very nice comment in the comments field. :)

[Dveditz (dveditz)]

Kai: is a patch based on comment 17 likely soon, and if so would it be small? I guess we won't 'block' the current release (since we're trying to wrap it up) but we'd really like to make this crash go away if we can.

[Kai Engert (kaie)]

(In reply to comment #21)
> Kai: is a patch based on comment 17 likely soon, and if so would it be small? I
> guess we won't 'block' the current release (since we're trying to wrap it up)
> but we'd really like to make this crash go away if we can.

We don't know if implementing what I mention in comment 17 will cure the crash.
It might only help us to understand more.

[Honzab-moz (honzab-moz)]

Created attachment 323866
derivation from nss shutdown object

Tested using firefox restart from add-ons dialog. This won't fix the crash.

[Chofmann (chofmann)]

I see this popping up to #6 on the topcrash list using a one day sample for RC2.

[Honzab-moz (honzab-moz)]

If anyone can reproduce this, please, list the JS callstack using xpc3250!DumpJSStack(). This prints it to stdout. This should help us.

[Kai Engert (kaie)]

Comment on attachment 323866
derivation from nss shutdown object

Honza, thanks for working on this patch.

Could you please follow the style that other code implementing nsNSSShutdownObject is using?

For example, please look at nsNSSCertCache.

In the destructor, you should use the locker, check for isAlreadyShutDown.

In virtualDestroyNSSReference all other code checks for isAlreadyShutDown, too. This prevents destroying objects that belong to a shutdown NSS session.

I would recommend you rearrance the code so that you have the same functions called virtualDestroyNSSReference, destructorSafeDestroyNSSReference. As a result, you'll have your calls to destroy the objects only once.

[Honzab-moz (honzab-moz)]

Created attachment 325550
derivation from nss shutdown object, 2

[Apport retracing service (apport)]

StacktraceTop:NSSRWLock_LockRead_Util () from /usr/lib/libnssutil3.so.1d
SECMOD_GetReadLock () from /usr/lib/libnss3.so.1d
PK11_GetAllTokens () from /usr/lib/libnss3.so.1d
PK11_GetBestSlotMultiple () from /usr/lib/libnss3.so.1d
PK11_GetBestSlot () from /usr/lib/libnss3.so.1d

[Beltzner (beltzner)]

Not blocking at this point, but topcrash+ (#1 topcrash on OSX) so let's see if we can get a safe patch in.

[Marco-zehe (marco-zehe)]

Got this crash on Windows once today:
bp-e34de6cf-428e-11dd-862f-001cc45a2ce4

[Samuel-sidler+old (samuel-sidler+old)]

Kai, does this patch look good? This is currently the #4 topcrash and we'd love to see it fixed in 3.0.2 (and, really 2.0.0.x).

[Dveditz (dveditz)]

Would be really nice to get this into 1.9.0.2/1.8.1.17

[Samuel-sidler+old (samuel-sidler+old)]

Not going to block on this for 1.9.0.2 (and likely 1.8.1.17). The patches aren't reviewed or landed and this will likely need bake time.

[Nelson-bolyard (nelson-bolyard)]

Note that Bug 450468 has a rather similar stack, but not identical.
That appears to be in PRNG code, while this bug is in hash code.

[Kai Engert (kaie)]

update: we still don't know what causes the crash, the patches in this bug are not a cure for the crash.

[Engamer01 (engamer01)]

I know this bug was originally filed as a Windows Vista bug, but I'm getting [@ NSSRWLock_LockRead_Util] crashes on Windows XP. Namely XP SP 3.

If it's alright, I can provide my crash reports (though I don't have any stack traces available =/ ). If I'm in the wrong place or you'd like a separate bug to be filed, do let me know.

[Samuel-sidler+old (samuel-sidler+old)]

Comment on attachment 314289
don't assume allocs won't fail and lock before reading

Clearing review request per comment 34.

[Nelson-bolyard (nelson-bolyard)]

I wonder if going off-line several seconds before shutting down the browser
would help.

[Honzab-moz (honzab-moz)]

Created attachment 387852
v2 for trunk, 1.9.1

Marking haveLoaded = false when we shutdown nss from nsNSSComponent. This really should prevent to get any psm component while nss is not at the moment initialized.

We should also apply the attachment 325550.

[Honzab-moz (honzab-moz)]

Created attachment 387853
v2 for 1.8.1

[Nelson-bolyard (nelson-bolyard)]

Bug 505223 is almost certainly a duplicate of this bug. It contains a stack showing that nsCryptoHash::Init is being called while in NS_ShutdownXPCOM_P .

[Honzab-moz (honzab-moz)]

*** Bug 505223 has been marked as a duplicate of this bug. ***

[Beltzner (beltzner)]

Kai: any ETA on a fix here? This is blocking our next release which we're trying to square away earlier rather than later.

[Samuel-sidler+old (samuel-sidler+old)]

Comment on attachment 384744
v1 for trunk [Checkin comment 73]

I'm assuming these two are obsolete...

[Honzab-moz (honzab-moz)]

Comment on attachment 325550
derivation from nss shutdown object, 2

(In reply to comment #86)
> (From update of attachment 384744 [details])
> I'm assuming these two are obsolete...

No, it is not, I point out in comment 81 we should also take it.

[Samuel-sidler+old (samuel-sidler+old)]

Uh... then we should really do a combined patch instead of separate ones...

[Dveditz (dveditz)]

Nominating this topcrash for 1.9.0.13 since it looks like these fixes will land on 1.9.1 before then.

[Kai Engert (kaie)]

Comment on attachment 325550
derivation from nss shutdown object, 2

This patch is good. It adds protection for the "NSS shutdown race" to the nsCryptoHash and nsCryptoHMAC objects, the same protection we already use for other PSM objects that store references to NSS objects.

r=kaie

[Kai Engert (kaie)]

Comment on attachment 387852
v2 for trunk, 1.9.1

r=kaie

[Honzab-moz (honzab-moz)]

Comment on attachment 387852
v2 for trunk, 1.9.1

http://hg.mozilla.org/mozilla-central/rev/4679ac688c56

[Honzab-moz (honzab-moz)]

Created attachment 391336
v2, as laned on m-c [Checkin comment 92]

[Honzab-moz (honzab-moz)]

Created attachment 391359
v1+v2 merged for 1.9.1

attachment 384744 and attachment 391336 folded and merged to 1.9.1.

[Honzab-moz (honzab-moz)]

Created attachment 391368
v1+v2 merged for 1.9.1 [Checkin comment 101]

This is the correct one...

[Samuel-sidler+old (samuel-sidler+old)]

Comment on attachment 391368
v1+v2 merged for 1.9.1 [Checkin comment 101]

This *just* landed on mozilla-central. I don't think we can take it in 1.9.1.2.

[Honzab-moz (honzab-moz)]

Created attachment 391397
v1+v2 merged for 1.9.0 [Checkin comment 102]

[Honzab-moz (honzab-moz)]

Created attachment 391554
v1+v2 merged for 1.8.1

[Dveditz (dveditz)]

Comment on attachment 391368
v1+v2 merged for 1.9.1 [Checkin comment 101]

Approved for 1.9.1.3, a=dveditz for release-drivers

[Dveditz (dveditz)]

Comment on attachment 391397
v1+v2 merged for 1.9.0 [Checkin comment 102]

Approved for 1.9.0.14, a=dveditz for release-drivers

[Honzab-moz (honzab-moz)]

Comment on attachment 391368
v1+v2 merged for 1.9.1 [Checkin comment 101]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/11bdfa7b7c58

[Honzab-moz (honzab-moz)]

Comment on attachment 391397
v1+v2 merged for 1.9.0 [Checkin comment 102]

Checking in security/manager/ssl/src/nsNSSComponent.cpp;
/cvsroot/mozilla/security/manager/ssl/src/nsNSSComponent.cpp,v <-- nsNSSComponent.cpp
new revision: 1.170; previous revision: 1.169
done
Checking in security/manager/ssl/src/nsNSSComponent.h;
/cvsroot/mozilla/security/manager/ssl/src/nsNSSComponent.h,v <-- nsNSSComponent.h
new revision: 1.55; previous revision: 1.54

[Micah Gersten (micahg)]

This is set to be released in Firefox 3.5.3 and Firefox 3.0.14

[Jruderman (jruderman)]

This fixed the topcrash; there have been no reports of crashes at NSSRWLock_LockRead_Util since July 25 builds.

http://crash-stats.mozilla.com/report/list?product=Firefox&version=Firefox%3A3.6a1pre&query_search=signature&query_type=exact&query=&date=&range_value=4&range_unit=weeks&do_query=1&signature=NSSRWLock_LockRead_Util

[Remkodekeijzer (remkodekeijzer)]

It still occurs in 3.5.2
Currently #8 in 3.5.2 topcrashes
http://crash-stats.mozilla.com/report/list?product=Firefox&version=Firefox%3A3.5.2&query_search=signature&query_type=exact&query=&date=&range_value=1&range_unit=weeks&do_query=1&signature=NSSRWLock_LockRead_Util

[Ryanvm (ryanvm)]

It was fixed for Gecko 1.9.1.3 per the flags above, meaning that Firefox 3.5.3 should no longer show the issue.

[Launchpad Janitor (janitor)]

This bug was fixed in the package firefox-3.0 - 3.0.14+build2+nobinonly-0ubuntu0.8.04.1

---------------
firefox-3.0 (3.0.14+build2+nobinonly-0ubuntu0.8.04.1) hardy-security; urgency=low

  * security/stability v3.0.14 build2 (FIREFOX_3_0_14_RELEASE)
    - see USN-821-1
    - fix LP: #236853 - firefox crashed with SIGSEGV in NSSRWLock_LockRead_Util()

 -- Alexander Sack <email address hidden> Mon, 07 Sep 2009 14:27:50 +0200

[Launchpad Janitor (janitor)]

This bug was fixed in the package firefox-3.5 - 3.5.3+build1+nobinonly-0ubuntu2

---------------
firefox-3.5 (3.5.3+build1+nobinonly-0ubuntu2) karmic; urgency=low

  * security/stability update v3.5.3 build1 (FIREFOX_3_5_3_BUILD1)
    - see USN-821-1
    - fix LP: #333127 - Firefox 3.5 and above crash on full screen flash video
    - fix LP: #236853 - firefox crashed with SIGSEGV in NSSRWLock_LockRead_Util()

  [ Alexander Sack <email address hidden> ]
  * fix LP: #422365 - apport hook fails because profiles_d is not initialized
    in add_info if no profiles.ini exist; we ensure that profiles_d gets instantiated
    as an empty map even if no profiles.ini exist.
    - update debian/apport/firefox-3.5.py
  * hook firefox-addons/searchplugins as the distribution/searchplugins
    directory to support localized distro search engines.
    - update debian/rules
  * in case localized search engines are available the main searchplugins
    directory is not scanned anymore; to fix this we provide a compatibility
    link /usr/lib/firefox-addons/searchplugins/common => /usr/lib/firefox-addons/searchplugins
    - update debian/firefox-3.5.links
  * fix localized search engine upstream code to properly deal with general.useragent.locale
    being a complex pref; also change plugin dir order to allow locale specific searchplugins
    to overlay the ones shipped in "searchplugins/common"
    - add debian/patches/fix_complex_locale_distro_searchplugins.patch
    - update debian/patches/series

  [ Jamie Strandboge <email address hidden> ]
  * add AppArmor profile (disabled by default) (LP: #382917)
    - debian/firefox-3.5.dirs: add etc/apparmor.d/disable
    - add debian/firefox-3.5.preinst.in: disable the profile on new installs
      and upgrades to this version
    - debian/firefox-3.5.postinst.in: reload profile
    - add debian/firefox-3.5.postrm.in: cleanup force-complain and disable
      directories
    - add debian/usr.bin.firefox.apparmor.in
    - debian/rules: install profile
    - add debian/README.Debian.in with note about AppArmor
    - debian/apport/firefox-3.5.py: add AppArmor information if the profile is
      not disabled
    - debian/firefox-3.5.preinst.in: allow for when apparmor is not installed

 -- Alexander Sack <email address hidden> Thu, 03 Sep 2009 10:03:08 +0200

[[e-mail] (leeshih49)]

JUST DO IT

[John Vivirito (gnomefreak)]

Please see https://bugzilla.mozilla.org/show_bug.cgi?id=427715#c105
it was fixed for 3.5
This should have been fixed in 3.0 see:
firefox-3.0 (3.0.14+build2+nobinonly-0ubuntu0.8.04.1) hardy-security; urgency=low

  * security/stability v3.0.14 build2 (FIREFOX_3_0_14_RELEASE)
    - see USN-821-1
    - fix LP: #236853 - firefox crashed with SIGSEGV in NSSRWLock_LockRead_Util()

 -- Alexander Sack <email address hidden> Mon, 07 Sep 2009 14:27:50 +0200

hardy and up has been fixed. please make sure you are at latest version and try using a new profile following the instructions on the following link:
https://wiki.ubuntu.com/MozillaTeam/Bugs#Try%20with%20a%20new%20profile

[John Vivirito (gnomefreak)]

Also please do not assign bugs to yourself unless you plan on fixing them.
when you reopen a bug please set it to incomplete or confirmed if it was confirmed.

Make sure you have hardy-security repos enabled

[John Vivirito (gnomefreak)]

Please do not assign yourself to bugs unless you plan on fixing it. thanks
Please see my comment above your last comment

[coachlaw10 (coachlaw10)]

how do I fix this problem?

[Micah Gersten (micahg)]

@Carl Cameron

Please do not change the status of Fix Released bugs.

[Micah Gersten (micahg)]

Please don't assign yourself bugs unless you are actually working on them. This fix has already been released.

[adsa_a (adsa-a)]

good!

[Micah Gersten (micahg)]

Please don't assign yourself bugs unless you are actually working on them. This fix has already been released.

[Honzab-moz (honzab-moz)]

Flag clean up.

[Honzab-moz (honzab-moz)]

Comment on attachment 391554
v1+v2 merged for 1.8.1

Flag clean up.