Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

nss: work around race condition in PK11_FindSlotByName #985

Closed
wants to merge 1 commit into from

Conversation

wangp
Copy link
Contributor

@wangp wangp commented Aug 26, 2016

Serialise the call to PK11_FindSlotByName in nss_create_object to avoid
spurious errors in a multi-threaded environment. The underlying cause
is a race condition in nssSlot_IsTokenPresent.

There are other paths into nssSlot_IsTokenPresent which may potentially
have the same problem, e.g. other calls to PK11_FindSlotByName, but this
is one path for which we have a test case.

Bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1297397

Serialise the call to PK11_FindSlotByName in nss_create_object to avoid
spurious errors in a multi-threaded environment. The underlying cause
is a race condition in nssSlot_IsTokenPresent.

There are other paths into nssSlot_IsTokenPresent which may potentially
have the same problem, e.g. other calls to PK11_FindSlotByName, but this
is one path for which we have a test case.

Bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1297397
@bagder bagder added the TLS label Aug 26, 2016
@bagder
Copy link
Member

bagder commented Aug 26, 2016

ping @kdudka, looks good to me, you agree? I'm sure we will have users on the NSS version with this bug for a long time so it is worth merging a private fix for it.

@kdudka
Copy link
Contributor

kdudka commented Aug 26, 2016

Thanks for the patch! It looks good to me. Just two remarks:

  • I would use a separate lock object for guarding PK11_FindSlotByName() to ease debugging.
  • We can guard all 3 calls to PK11_FindSlotByName(). Is there any downside of doing that?

@kdudka kdudka self-assigned this Aug 26, 2016
@wangp
Copy link
Contributor Author

wangp commented Aug 26, 2016

Ok, I will make those changes next week.

@kdudka
Copy link
Contributor

kdudka commented Aug 26, 2016

@wangp No problem, I can make the changes on your behalf as long as you are fine with the proposal.

@wangp
Copy link
Contributor Author

wangp commented Aug 26, 2016

Yes, go ahead. I won't have time until next Wednesday.

@kdudka kdudka closed this in 3a5d5de Aug 26, 2016
@lock lock bot locked as resolved and limited conversation to collaborators Jan 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Development

Successfully merging this pull request may close these issues.

None yet

3 participants