ShutdownCRLCache doesn't NULL pointers

RESOLVED FIXED in 3.6.1

Status

NSS
Libraries
P1
normal
RESOLVED FIXED
15 years ago
15 years ago

People

(Reporter: Julien Pierre, Assigned: Julien Pierre)

Tracking

Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Assignee)

Description

15 years ago
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 .
(Assignee)

Updated

15 years ago
Priority: -- → P1
Target Milestone: --- → 3.6.1
(Assignee)

Comment 1

15 years ago
Created attachment 104797 [details] [diff] [review]
NULL pointers upon destruction
(Assignee)

Updated

15 years ago
Blocks: 171331

Updated

15 years ago
Blocks: 174647
(Assignee)

Comment 2

15 years ago
Taking bug.
Assignee: wtc → jpierre
(Assignee)

Comment 3

15 years ago
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
Status: NEW → RESOLVED
Last Resolved: 15 years ago
Resolution: --- → FIXED

Comment 4

15 years ago
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;
(Assignee)

Comment 5

15 years ago
Reopening per Wan-Teh's comments.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 6

15 years ago
Created attachment 105388 [details] [diff] [review]
improve handling of initialization / shutdown of the CRL cache using a static status variable

additional patch
(Assignee)

Comment 7

15 years ago
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
Status: REOPENED → RESOLVED
Last Resolved: 15 years ago15 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.