177798 - ShutdownCRLCache doesn't NULL pointers

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P1)

Description

[Julien Pierre]

The CRL cache uses a PRLock* and a hash table of issuers.
They are initialized in InitCRLCache and destroyed in ShutdownCRLCache, which
are called respectively upon NSS initialization and shutdown.
However, ShutdownCRLCache doesn't NULL the pointers after the destruction.
If an application restarts NSS (eg. browser switching profile), then when
InitCRLCache gets called the 2nd time around, it will not reallocate a lock or a
hash table, since the pointers are still set. This will cause crash in the
application during cert verification, or upon shutdown when trying to free them.
The fix for this needs to into both 3.6.1 and 3.7 .

Comment 1

[Julien Pierre]

Attachment: NULL pointers upon destruction

Comment 2

[Julien Pierre]

Taking bug.

Comment 3

[Julien Pierre]

Checked in to NSS_3_6_BRANCH :
Checking in crl.c;
/cvsroot/mozilla/security/nss/lib/certdb/crl.c,v  <--  crl.c
new revision: 1.26.2.2; previous revision: 1.26.2.1
done

And the tip ( NSS 3.7 ) :

Checking in crl.c;
/cvsroot/mozilla/security/nss/lib/certdb/crl.c,v  <--  crl.c
new revision: 1.29; previous revision: 1.28
done

Comment 4

[Wan-Teh Chang]

While I think these are good changes, the root cause
of the bug is that InitCRLCache returns SECSuccess
without doing anything if crlcache.lock is not null.
I think the "if (!crlcache.lock)" check in InitCRLCache
is not necessary and should be deleted.  Alternatively,
it should be replaced by either an assertion:

    PORT_Assert(!crlcache.lock);

or proper error handling:

    if (crlcache.lock) {
        PORT_SetError(<some error code>);
        return SECFailure;
    }

By the way, this statement in InitCRLCache

     crlcache.lock = PR_FALSE;

should be

     crlcache.lock = NULL;

Comment 5

[Julien Pierre]

Reopening per Wan-Teh's comments.

Comment 6

[Julien Pierre]

Attachment: improve handling of initialization / shutdown of the CRL cache using a static status variable

additional patch

Comment 7

[Julien Pierre]

Discussed this with Bob. The new patch is going into the tip, not in NSS_3_6_BRANCH.

Checking in crl.c;
/cvsroot/mozilla/security/nss/lib/certdb/crl.c,v  <--  crl.c
new revision: 1.30; previous revision: 1.29
done