curl-library
Re: problem with NSS backend and threads
Date: Mon, 22 Aug 2016 15:27:23 +0200
On Thursday, August 18, 2016 11:45:24 Peter Wang wrote:
> On Wed, 17 Aug 2016 10:42:38 +0200, Kamil Dudka <kdudka_at_redhat.com> wrote:
> > Do I understand it correctly that PK11_FindSlotByName(slot_name) returns
> > NULL? What is the output of PR_GetError() in that case?
>
> Hi,
>
> Yes, PK11_FindSlotByName() returns NULL.
> PR_GetError() returns -8127 SEC_ERROR_NO_TOKEN.
>
> Upon further investigation, PK11_FindSlotByName() returns NULL
> ... because PK11_IsPresent(tmpSlot) returns false
> ... because pk11_IsPresentCertLoad(slot,PR_TRUE) returns false
> ... because nssToken_IsPresent(slot->nssToken) returns false
> ... because nssSlot_IsTokenPresent(token->slot) returns false
>
> which is due to this branch in nssSlot_IsTokenPresent()
>
> /* avoid repeated calls to check token status within set interval */
> if (within_token_delay_period(slot)) {
> return ((slot->ckFlags & CKF_TOKEN_PRESENT) != 0);
> }
>
> Peter
Thank you for analyzing the cause of it! After having a brief look at the
code, I believe the race condition is obvious:
PK11_FindSlotByName() acquires only a "read" lock, which does not ensure
mutual exclusion of readers. So multiple calls of PK11_FindSlotByName()
can be executed in parallel. As there is no additional lock on the way,
nssSlot_IsTokenPresent() can also be executed in parallel.
If there is a schedule where one thread has assigned slot->lastTokenPing
but not yet set the CKF_TOKEN_PRESENT flag in slot->ckFlags, all other
threads will keep returning SEC_ERROR_NO_TOKEN (without actually checking
presence of the token) until the first thread continues (or the specified
timeout elapses).
In order to fix it, we need to add a mutex to nssSlot_IsTokenPresent() or
at least reorder mutation of slot's state such that slot->lastTokenPing is
not updated before the CKF_TOKEN_PRESENT flag in slot->ckFlags is updated.
Could you please file it as a bug against NSS?
https://bugzilla.mozilla.org/enter_bug.cgi#h=bugForm|NSS
... or should I file the bug on your behalf?
Kamil
-------------------------------------------------------------------
List admin: https://cool.haxx.se/list/listinfo/curl-library
Etiquette: https://curl.haxx.se/mail/etiquette.html
Received on 2016-08-22