Note: There are a few cases of duplicates in user autocompletion which are being worked on.

cache->searchedObjectType[] should always be cleared in clear_cache()

RESOLVED FIXED in 3.9.3

Status

NSS
Libraries
P2
normal
RESOLVED FIXED
13 years ago
13 years ago

People

(Reporter: kinmoz, Assigned: Nelson Bolyard (seldom reads bugmail))

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

1.12 KB, patch
Nelson Bolyard (seldom reads bugmail)
: review+
Details | Diff | Splinter Review
(Reporter)

Description

13 years ago
In clear_cache() there is a for loop that looks something like this:

    for (objectType = cachedCerts; objectType <= cachedCRLs; objectType++) {
        if (!cache->objects[objectType]) {
            continue;
        }

        ...

        cache->objects[objectType] = NULL;
        cache->searchedObjectType[objectType] = PR_FALSE;
    }

It seems that it is possible to get into the situation where
cache->objects[objectType] is NULL while cache->searchedObjectType[objectType]
is PR_TRUE.

In that particular case clear_cache() will never set the
cache->searchedObjectType[objectType] flag to PR_FALSE, which could result in
get_token_certs_for_cache() never loading the certs for new tokens that are
inserted into the slot because it thinks the certs were already cached off of
the token.

My proposed fix is to set cache->searchedObjectType[objectType] at the top of
the for loop.
(Reporter)

Comment 1

13 years ago
Created attachment 159648 [details] [diff] [review]
Patch Rev 1 (against the NSS_3_9_BRANCH)

I probably should've mentioned the file these functions live in ... doh ...
You'll find the cache functions mentioned above in
mozilla/security/nss/lib/dev/devutil.c.
(Assignee)

Comment 2

13 years ago
Comment on attachment 159648 [details] [diff] [review]
Patch Rev 1 (against the NSS_3_9_BRANCH)

r=nelson
Attachment #159648 - Flags: review+
(Assignee)

Comment 3

13 years ago
Checked in on NSS 3.9 branch for NSS 3.9.3
Checking in dev/devutil.c;  new revision: 1.23.16.1; previous revision: 1.23
Assignee: wchang0222 → nelson
(Assignee)

Comment 4

13 years ago
Checked in on trunk
Checking in devutil.c;  new revision: 1.25; previous revision: 1.24
Status: NEW → RESOLVED
Last Resolved: 13 years ago
Priority: -- → P2
Resolution: --- → FIXED
Target Milestone: --- → 3.9.3
Version: 3.9.3 → 3.9.2

Comment 5

13 years ago
Comment on attachment 159648 [details] [diff] [review]
Patch Rev 1 (against the NSS_3_9_BRANCH)

r=wtc.

I reviewed create_object_array and found that
it may return NULL and set *status to PR_SUCCESS
(and *numObjects to 0).  So get_token_certs_for_cache
may set cache->objects[cachedCerts] to NULL and
cache->searchedObjectType[cachedCerts] to PR_TRUE.
Therefore clear_cache needs to handle this properly.

(Another way for a cache to get into this state is
via nssTokenObjectCache_RemoveObject, which may set
cache->objects[oType] to NULL without modifying
cache->searchedObjectType[oType].)
You need to log in before you can comment on or make changes to this bug.