When NSSTrustDomain_FindTokenByName() returns 'tok', what guarantees that it still exists, the moment after the locks are dropped?
Aren't we supposed to use nssToken_AddRef() before dropping the locks,
to ensure that it stays around long enough for our caller to actually
use it?
So we'd want something like the patch below?
I ask because we're adding a new NSSTrustDomain_FindTokensByURI()
function and won't want to make the same mistake. I've been drumming
into Varun that he needs to make his new functions look at much like
*existing* similar functions as possible... but perhaps *not* to the
extent of preserving their bugs :)
diff --git a/lib/pk11wrap/pk11cert.c b/lib/pk11wrap/pk11cert.c
index 1b9be63..e4443c5 100644
--- a/lib/pk11wrap/pk11cert.c
+++ b/lib/pk11wrap/pk11cert.c
@@ -731,20 +731,23 @@ find_certs_from_nickname(const char *nickname, void
*wincx)
if ((delimit = PORT_Strchr(nickCopy,':')) != NULL) {
tokenName = nickCopy;
nickname = delimit + 1;
*delimit = '\0';
/* find token by name */
+ /* We got a ref on the token here... */
token = NSSTrustDomain_FindTokenByName(defaultTD, (NSSUTF8 *)tokenName);
if (token) {
slot = PK11_ReferenceSlot(token->pk11slot);
} else {
PORT_SetError(SEC_ERROR_NO_TOKEN);
}
*delimit = ':';
} else {
slot = PK11_GetInternalKeySlot();
token = PK11Slot_GetNSSToken(slot);
+ /* ... so we need one here too... */
+ nssToken_AddRef(token);
}
if (token) {
nssList *certList;
nssCryptokiObject **instances;
nssTokenSearchType tokenOnly = nssTokenSearchType_TokenOnly;
@@ -803,10 +806,14 @@ find_certs_from_nickname(const char *nickname, void
*wincx)
}
loser:
if (collection) {
nssPKIObjectCollection_Destroy(collection);
}
+ if (token) {
+ /* ... and we drop it here */
+ nssToken_Destroy(token);
+ }
if (slot) {
PK11_FreeSlot(slot);
}
if (nickCopy) PORT_Free(nickCopy);
return certs;
diff --git a/lib/pki/trustdomain.c b/lib/pki/trustdomain.c
index 87ea6da..dd72347 100644
--- a/lib/pki/trustdomain.c
+++ b/lib/pki/trustdomain.c
@@ -294,11 +294,14 @@ NSSTrustDomain_FindTokenByName (
tok != (NSSToken *)NULL;
tok = (NSSToken *)nssListIterator_Next(td->tokens))
{
if (nssToken_IsPresent(tok)) {
myName = nssToken_GetName(tok);
- if (nssUTF8_Equal(tokenName, myName, &nssrv)) break;
+ if (nssUTF8_Equal(tokenName, myName, &nssrv)) {
+ tok = nssToken_AddRef(tok);
+ break;
+ }
}
}
nssListIterator_Finish(td->tokens);
NSSRWLock_UnlockRead(td->tokensLock);
return tok;
(Note that there is no find_certs_from_nickname() in HEAD at the
moment; there are two almost identical copies of the same code,
duplicated in PK11_FindCertsFromNickname() and
PK11_FindCertFromNickname() with the occasional bug fix applied to one
but not the other. I fixed that already in my tree and will submit that
cleanup separately. Again, I didn't want Varun doing the same thing
with the equivalent FromURI() functions.)
--
David Woodhouse Open Source Technology Centre
david.woodho...@intel.com Intel Corporation
smime.p7s
Description: S/MIME cryptographic signature
-- dev-tech-crypto mailing list dev-tech-crypto@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-tech-crypto
