1663661 - Crash in [@ nssToken_IsPresent | PK11_TraverseCertsInSlot]

Status: RESOLVED WORKSFORME

(NSS :: Libraries, defect, P1)

Description

[Wayne Mery (:wsmwk)]

Crash is new in version 80. #20 crash for 81 beta bp-8d989e12-be13-4d16-9821-6fe5f0200908

Crash report: https://crash-stats.mozilla.org/report/index/a263bf33-4fc1-4145-9936-f70680200812 version 80

Top 10 frames of crashing thread:

0 nss3.dll nssToken_IsPresent security/nss/lib/dev/devtoken.c:1427
1 nss3.dll PK11_TraverseCertsInSlot security/nss/lib/pk11wrap/pk11cert.c:2308
2 nss3.dll PK11_ListCertsInSlot security/nss/lib/pk11wrap/pk11cert.c:2845
3 xul.dll IntermediatePreloadingHealerCallback security/manager/ssl/nsNSSComponent.cpp:2192
4 xul.dll nsTimerImpl::Fire xpcom/threads/nsTimerImpl.cpp:562
5 xul.dll nsTimerEvent::Run xpcom/threads/TimerThread.cpp:251
6 xul.dll mozilla::TaskQueue::Runner::Run xpcom/threads/TaskQueue.cpp:158
7 xul.dll nsThreadPool::Run xpcom/threads/nsThreadPool.cpp:299
8 xul.dll nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:1234
9 xul.dll NS_ProcessNextEvent xpcom/threads/nsThreadUtils.cpp:513

Comment 1

[Kai Engert [:KaiE:]]

Looks like a new bug introduced in NSS 3.55 ?

Comment 2

[Kai Engert [:KaiE:]]

Or was it introduced as part of IntermediatePreloadingHealerCallback from bug 1630434 ?

Comment 3

[Dana Keeler (she/her) (use needinfo) [:keeler]]

It's from bug 1630434, but it indicates an underlying race condition in NSS.

Comment 4

[Kevin Jacobs [:kjacobs]]

This is the same crash as in bug 1650654. It occurs in nssSlot_IsTokenPresent when slot->token is NULL. [0] and [1] imply that slot->token may be set to NULL under normal course of execution (mostly, but maybe not only during shutdown).

I see two issues in nssSlot_IsTokenPresent:

1. slot->lock is not held when accessing slot->token, as it is in [0] and [1].
2. There are some paths which which will dereference slot->token, even if it is NULL [2].

I'll attach a patch soon.

[0] https://searchfox.org/mozilla-central/source/security/nss/lib/dev/devtoken.c#36-40
[1] https://searchfox.org/mozilla-central/source/security/nss/lib/dev/devslot.c#286-291
[2] https://searchfox.org/mozilla-central/source/security/nss/lib/dev/devslot.c#173,176,183,214,240-241

Comment 5

[Kevin Jacobs [:kjacobs]]

Attachment: Bug 1663661 - Guard against NULL token in nssSlot_IsTokenPresent.

Comment 6

[Kevin Jacobs [:kjacobs]]

https://hg.mozilla.org/projects/nss/rev/0ed11a5835ac1556ff978362cd61069d48f4c5db

Comment 7

[Thomas Deutschmann]

The bugfix is causing a regression, see bug 1679290.

Comment 8

[Kevin Jacobs [:kjacobs]]

This patch has been reverted after two problems with various third-party PKCS11 modules. Nested slot/session locking caused most of the reports, but some slow devices seem to cause Fx hangs when holding the slot lock for a longer duration. It's not clear if the root cause of this is in NSS or Fx, but without hardware to reproduce and test, there's little that we can do.

Any future attempts at fixing this need much more thorough testing. The only PKCS11 device I have access to (a SafeNet eToken 5110) could never reproduce. Most reports came from Gemalto and Oberthur smartcards in Italy and Brazil.

An additional note: Enabling OSClientCerts in Firefox resolved all incompatibilities in the original patch. That's good, but it may have caused us to miss early bug reports since it's enabled by default in Nightly and Early Beta. In general though, we seem to get little pre-release feedback from users that rely on these devices.

https://hg.mozilla.org/projects/nss/rev/97ef009f7a782ec6e114255e3ca6ec78859d58bc

Comment 9

[Wayne Mery (:wsmwk)]

FWIW, the Thunderbird crashes are gone since Oct 16, 2020/buildid 20200929195155

Comment 10

[BugBot [:suhaib / :marco/ :calixte]]

Closing because no crashes reported for 12 weeks.