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)
NSS
Libraries
Tracking
(Not tracked)
RESOLVED
FIXED
3.12.11
People
(Reporter: briansmith, Unassigned)
Details
Attachments
(1 file, 1 obsolete file)
1.10 KB,
patch
|
rrelyea
:
review+
|
Details | Diff | Splinter Review |
See attached patch. Note that OCSP_Global.maxCacheEntries might be negative.
Attachment #530768 -
Flags: review?(rrelyea)
Comment 1•14 years ago
|
||
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+
Comment 2•14 years ago
|
||
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.
Comment 3•14 years ago
|
||
Attachment #543482 -
Flags: review?(rrelyea)
Reporter | ||
Updated•14 years ago
|
Attachment #530768 -
Attachment is obsolete: true
Reporter | ||
Comment 4•14 years ago
|
||
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 5•14 years ago
|
||
Comment on attachment 543482 [details] [diff] [review]
Patch v2
r+ rrelyea
Attachment #543482 -
Flags: review?(rrelyea) → review+
Updated•14 years ago
|
Summary: signed/unsigned comparison in ocsp.c → signed/unsigned comparison in ocsp.c; risk of keeping monitor locked
Comment 6•14 years ago
|
||
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.
Description
•