Last Comment Bug 177798 - ShutdownCRLCache doesn't NULL pointers
: ShutdownCRLCache doesn't NULL pointers
Product: NSS
Classification: Components
Component: Libraries (show other bugs)
: 3.6
: All All
: P1 normal (vote)
: 3.6.1
Assigned To: Julien Pierre
: Bishakha Banerjee
Depends on:
Blocks: 171331 1.2
  Show dependency treegraph
Reported: 2002-10-31 13:59 PST by Julien Pierre
Modified: 2002-11-06 16:12 PST (History)
5 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---

NULL pointers upon destruction (552 bytes, patch)
2002-10-31 14:00 PST, Julien Pierre
no flags Details | Diff | Splinter Review
improve handling of initialization / shutdown of the CRL cache using a static status variable (3.32 KB, patch)
2002-11-06 16:09 PST, Julien Pierre
no flags Details | Diff | Splinter Review

Description Julien Pierre 2002-10-31 13:59:13 PST
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 2002-10-31 14:00:24 PST
Created attachment 104797 [details] [diff] [review]
NULL pointers upon destruction
Comment 2 Julien Pierre 2002-11-01 16:16:28 PST
Taking bug.
Comment 3 Julien Pierre 2002-11-01 16:18:46 PST
Checked in to NSS_3_6_BRANCH :
Checking in crl.c;
/cvsroot/mozilla/security/nss/lib/certdb/crl.c,v  <--  crl.c
new revision:; previous revision:

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
Comment 4 Wan-Teh Chang 2002-11-03 15:40:01 PST
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:


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 2002-11-06 16:03:50 PST
Reopening per Wan-Teh's comments.
Comment 6 Julien Pierre 2002-11-06 16:09:56 PST
Created attachment 105388 [details] [diff] [review]
improve handling of initialization / shutdown of the CRL cache using a static status variable

additional patch
Comment 7 Julien Pierre 2002-11-06 16:12:50 PST
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

Note You need to log in before you can comment on or make changes to this bug.