Closed Bug 655411 Opened 14 years ago Closed 14 years ago

signed/unsigned comparison in ocsp.c; risk of keeping monitor locked

Categories

(NSS :: Libraries, defect)

defect
Not set
trivial

Tracking

(Not tracked)

RESOLVED FIXED
3.12.11

People

(Reporter: briansmith, Unassigned)

Details

Attachments

(1 file, 1 obsolete file)

See attached patch. Note that OCSP_Global.maxCacheEntries might be negative.
Attachment #530768 - Flags: review?(rrelyea)
Comment on attachment 530768 [details] [diff] [review] Fix signed/unsigned warning in ocsp.c r+ with a few caveats. Good catch that OCSP_Global.maxCacheEntries can be negative. Clearly it's not going to be negative in our while loop, unless ocsp_RemoveCacheItem changes it because we have a test which bounces out on the line above, so the PR_MAX is actually redundant. NOTE: there looks like a monitor issue if the cache is disabled or unlimited. We don't all PR_ExitMonitory(). We should fix that while we are here.. bob
Attachment #530768 - Flags: review?(rrelyea) → review+
Brian, I agree the type cast is good. But I think PR_MAX is unnecessary. Before we arrive at that line, we have already checked that OCSP_Global.maxCacheEntries is positive. We hold the lock that protects this variable, so it cannot become negative while we are executing the loop.
Attached patch Patch v2Splinter Review
Attachment #543482 - Flags: review?(rrelyea)
Attachment #530768 - Attachment is obsolete: true
I think this should go into 3.12.11 because of the monitor issue that Kai's patch fixes.
Target Milestone: --- → 3.12.11
Comment on attachment 543482 [details] [diff] [review] Patch v2 r+ rrelyea
Attachment #543482 - Flags: review?(rrelyea) → review+
Summary: signed/unsigned comparison in ocsp.c → signed/unsigned comparison in ocsp.c; risk of keeping monitor locked
3.12 branch: Checking in ocsp.c; /cvsroot/mozilla/security/nss/lib/certhigh/ocsp.c,v <-- ocsp.c new revision: 1.65.2.1; previous revision: 1.65 trunk: Checking in ocsp.c; /cvsroot/mozilla/security/nss/lib/certhigh/ocsp.c,v <-- ocsp.c new revision: 1.66; previous revision: 1.65
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: