Closed
Bug 1381784
Opened 4 years ago
Closed 4 years ago
deadlock in nssSlot_IsTokenPresent, if it is called from nssSlot_GetToken
Categories
(NSS :: Libraries, defect)
NSS
Libraries
Tracking
(firefox-esr52 unaffected, firefox55 fixed)
RESOLVED
FIXED
3.31
Tracking | Status | |
---|---|---|
firefox-esr52 | --- | unaffected |
firefox55 | --- | fixed |
People
(Reporter: ueno, Assigned: ueno)
References
Details
(Keywords: regression)
Attachments
(1 file)
1.15 KB,
patch
|
ttaubert
:
review+
|
Details | Diff | Splinter Review |
The recent commit: https://hg.mozilla.org/projects/nss/rev/859a4f82207b changed nssSlot_GetToken() to hold a lock when calling nssSlot_IsTokenPresent(), while the function also takes the same lock: https://hg.mozilla.org/projects/nss/rev/859a4f82207b#l1.33 https://hg.mozilla.org/projects/nss/file/tip/lib/dev/devslot.c#l134 The attached patch moves the locking below to avoid possible deadlock. Originally reported by Kostya Vasilyev in: https://bugzilla.redhat.com/show_bug.cgi?id=1470352#c8
Attachment #8887427 -
Flags: review?(ttaubert)
Attachment #8887427 -
Flags: review?(dkeeler)
Comment 1•4 years ago
|
||
Comment on attachment 8887427 [details] [diff] [review] devslot-lock.patch Review of attachment 8887427 [details] [diff] [review]: ----------------------------------------------------------------- LGTM with the nit fixed. Thanks! ::: lib/dev/devslot.c @@ +231,5 @@ > + /* Even if a token should be present, check `slot->token` too as it > + * might be gone already. This would happen mostly on shutdown. */ > + nssSlot_EnterMonitor(slot); > + if (slot->token) > + rvToken = nssToken_AddRef(slot->token); Please use braces even for a single-line in the body of the if-statement.
Attachment #8887427 -
Flags: review?(ttaubert) → review+
Comment on attachment 8887427 [details] [diff] [review] devslot-lock.patch Review of attachment 8887427 [details] [diff] [review]: ----------------------------------------------------------------- LGTM (clearing r? because I'm not an NSS peer - Tim's r+ should be sufficient, I believe.)
Attachment #8887427 -
Flags: review?(dkeeler)
Comment 3•4 years ago
|
||
Thanks Daiki! https://hg.mozilla.org/projects/nss/rev/58026f3ade78 Do we expect this to affect many users? Does this justify a NSS 3.31.x release, or can this wait for 3.32 which will probably get release around August 3rd?
Assignee: nobody → dueno
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → 3.32
Comment 4•4 years ago
|
||
(In reply to Kai Engert (:kaie:) (on vacation) from comment #3) > Does this justify a NSS 3.31.x release, or can this wait for 3.32 which will > probably get release around August 3rd? Looks like it does... https://hg.mozilla.org/projects/nss/rev/8dd4fb083510
Target Milestone: 3.32 → 3.31
Updated•4 years ago
|
Blocks: 1273678
status-firefox55:
--- → fixed
status-firefox-esr52:
--- → unaffected
Keywords: regression
You need to log in
before you can comment on or make changes to this bug.
Description
•