crash in [@ NSSRWLock_LockRead] closing Mozilla after open Master Password pref panel

VERIFIED FIXED in 3.9.3

Status

NSS
Libraries
P1
critical
VERIFIED FIXED
13 years ago
13 years ago

People

(Reporter: Andrew Schultz, Assigned: Wan-Teh Chang)

Tracking

({crash, regression, topcrash})

3.9.3
3.9.3
crash, regression, topcrash

Firefox Tracking Flags

(Not tracked)

Details

(crash signature, URL)

Attachments

(2 attachments)

(Reporter)

Description

13 years ago
If I just open Mozilla, open the Master Password pref panel and then close
Mozilla (this is with a profile without any stored passwords), it crashes:

#6  0x027a180d in NSSRWLock_LockRead (rwlock=0x0) at nssrwlk.c:209
#7  0x0277b388 in SECMOD_GetReadLock (modLock=0x0) at pk11list.c:73
#8  0x027740e7 in PK11_LogoutAll () at pk11slot.c:1259
#9  0x04735a60 in nsSecretDecoderRing::LogoutAndTeardown (this=0x89fdbf0) at
nsSDR.cpp:354
#10 0x02728767 in WLLT_ExpirePassword (status=0x8135548) at wallet.cpp:725
#11 0x02721f9e in nsWalletlibService::Observe (this=0x88a7d50, aSubject=0x82c2938, 
    aTopic=0x6bce58 "profile-before-change", someData=0x6bd130) at
nsWalletService.cpp:211
#12 0x0040a9c1 in nsObserverService::NotifyObservers (this=0x82455e0,
aSubject=0x82c2938, 
    aTopic=0x6bce58 "profile-before-change", someData=0x6bd130) at
nsObserverService.cpp:208
#13 0x006a7c55 in nsProfile::ShutDownCurrentProfile (this=0x82c2938,
shutDownType=1) at nsProfile.cpp:1364
#14 0x08062095 in DoOnShutdown () at nsAppRunner.cpp:807
#15 0x0806445b in main (argc=3, argv=0xfef1cfe4) at nsAppRunner.cpp:1803

this regressed between linux trunk 2004100806 and 2004100907, but backing out
bug 261267 and bug 262689 didn't seem to help.
(Reporter)

Comment 1

13 years ago
this looks like it might be an NSS bug
(Reporter)

Comment 2

13 years ago
talkback says this affects Mac and Win as well.
Keywords: topcrash
OS: Linux → All
Hardware: PC → All
Summary: crash closing Mozilla after open Master Password pref panel → crash in [@ NSSRWLock_LockRead] closing Mozilla after open Master Password pref panel
(Assignee)

Comment 3

13 years ago
I confirmed this is a known NSS bug in the current
NSS_CLIENT_TAG (which I updated last Friday 2004-10-08
around 5 PM US Pacific Daylight Time).

Bob Relyea will implement a fix and we will get the
fix into NSS_CLIENT_TAG as soon as possible.
(Assignee)

Comment 4

13 years ago
Changed product to NSS.
Assignee: dveditz → wchang0222
Component: Password Manager → Libraries
Product: Browser → NSS
QA Contact: bishakhabanerjee
Target Milestone: --- → 3.9.3
Version: Trunk → 3.9.3

Comment 5

13 years ago
In some sense this is a latent bug in the application. It's calling NSS
functions even when NSS has not been initialized. Since PK11_LogoutAll has up to
this point been accidentally safe, NSS, at the very least, should continue to
allow calls to PK11_LogoutAll to occur even if NSS is not initialized (in this
case it will continue to be a noop). This will allow bug for bug binary
compatibility with previous versions of NSS. I'll attach a patch to make it so..

bob

Comment 6

13 years ago
Created attachment 161790 [details] [diff] [review]
NSS 3.9 Branch: Make PK11LogoutAll safe even if NSS not initialized.

Comment 7

13 years ago
Created attachment 161791 [details] [diff] [review]
NSS Trunk: Make PK11LogoutAll safe even if NSS not initialized

Comment 8

13 years ago
There are two patches mostly because the function moved from one file to another
between the NSS 3.9 Branch and the current trunk build.

Comment 9

13 years ago
Comment on attachment 161790 [details] [diff] [review]
NSS 3.9 Branch: Make PK11LogoutAll safe even if NSS not initialized.

wtc please review the patch.
Attachment #161790 - Flags: review?(wchang0222)

Comment 10

13 years ago
Comment on attachment 161791 [details] [diff] [review]
NSS Trunk: Make PK11LogoutAll safe even if NSS not initialized

Same here (patch is identical, file location has changed).
Attachment #161791 - Flags: review?(wchang0222)
(Assignee)

Comment 11

13 years ago
Comment on attachment 161790 [details] [diff] [review]
NSS 3.9 Branch: Make PK11LogoutAll safe even if NSS not initialized.

r=wtc.

I reproduced the crash and verified that this patch
fixed the crash.

Bob, please check this patch in on the trunk and
the NSS_3_9_BRANCH.
Attachment #161790 - Flags: review?(wchang0222) → review+
(Assignee)

Updated

13 years ago
Attachment #161791 - Flags: review?(wchang0222) → review+
(Assignee)

Comment 12

13 years ago
Bob, do you think we should fix the application, too?
Priority: -- → P1

Comment 13

13 years ago
I think it would be a good idea to bracket the calls with

if (NSS_IsInitialized()) {
}

in code fragments that are trying shutdown NSS even if it wasn't already shutdown.

bob
(Assignee)

Comment 14

13 years ago
Comment on attachment 161790 [details] [diff] [review]
NSS 3.9 Branch: Make PK11LogoutAll safe even if NSS not initialized.

Bob checked in the fix on the NSS trunk
(NSS 3.10) and NSS_3_9_BRANCH (NSS 3.9.3).

I've checked in this patch in the
NSS_CLIENT_TAG for the Mozilla trunk.
(Assignee)

Updated

13 years ago
Status: NEW → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED
(Assignee)

Comment 15

13 years ago
Andrew, could you verify this fix with the latest trunk
build?
(Reporter)

Comment 16

13 years ago
verified fixed with linux trunk 2004101205

I filed bug 264096 to implement Robert's suggestions for things calling NSS
functions
Status: RESOLVED → VERIFIED
Crash Signature: [@ NSSRWLock_LockRead]
You need to log in before you can comment on or make changes to this bug.