OOM crash [@ nssArena_Destroy - nssTrustDomain_TraverseCertificatesBySubject][@ nssArena_Destroy - nssTrustDomain_TraverseCertificatesByNickname] Dereferencing possibly NULL "tmpArena"

RESOLVED FIXED in 3.11.3

Status

NSS
Libraries
P2
normal
RESOLVED FIXED
12 years ago
7 years ago

People

(Reporter: timeless, Assigned: Alexei Volkov)

Tracking

({coverity, crash})

3.11
3.11.3
All
Linux
coverity, crash

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [CID 308 309], crash signature, URL)

Attachments

(1 attachment)

1.60 KB, patch
Nelson Bolyard (seldom reads bugmail)
: review+
Julien Pierre
: superreview+
Details | Diff | Splinter Review
(Reporter)

Description

12 years ago
Event returned_null: Function "NSSArena_Create" returned NULL value (checked 12 out of 14 times) [model]
Hardware: PC → All
Target Milestone: --- → 3.11.2
Priority: -- → P2
(Assignee)

Updated

12 years ago
Assignee: nobody → alexei.volkov.bugs
(Assignee)

Comment 1

12 years ago
Looks like these functions are obsolete, or at least lxr does not show that anybody uses them and they are not a part of public interface.

Also, functions create NSSCertificate ref leak, if callback function does not
call DestroyCertificate for every cert it is called with.
Any functions with long prefix names, such as nssArena_  or nssTrustDomain_
are NEW code that was written as part of project Stan, which has not yet 
been put into use.  That is, the new functions were written to be used,
but no other code yet calls them.  

Perhaps we should make all this code be #ifdef STAN.
I don't want to delete it, because it will be forgotten and not ever used.
But we shouldn't be wasting time fixing bugs in dead code.
(Reporter)

Comment 3

12 years ago
i don't mind the ifdef route, but if you go that way, please be sure to include a visible comment indicating that the code is known to have leaks and crashes which should be addressed before it's recomissioned :).
ok, #ifdef STAN_CODE_WITH_OOM_CRASHES    :)
CID 309, possibly others too.
(Assignee)

Updated

12 years ago
Whiteboard: [CID 308 309]

Comment 6

12 years ago
Retargetting all P2s to 3.11.3 .
Target Milestone: 3.11.2 → 3.11.3
(Assignee)

Comment 7

12 years ago
Lowing priority. The code is not used yet.
Severity: critical → normal
(Assignee)

Comment 8

12 years ago
Created attachment 235130 [details] [diff] [review]
check arena value after creation

Prefer to fix the function instead of ifdef-ing them.

Even though arena argument is optional for NSSTrustDomain_FindCertificatesBySubject and NSSTrustDomain_FindCertificatesByName, looks like it was meant have successful arena allocation before proceeding to next call.
Attachment #235130 - Flags: review?(nelson)
Comment on attachment 235130 [details] [diff] [review]
check arena value after creation

r=nelson
Attachment #235130 - Flags: review?(nelson) → review+
(Assignee)

Comment 10

12 years ago
Comment on attachment 235130 [details] [diff] [review]
check arena value after creation

supper review for 3.11 branch
Attachment #235130 - Flags: superreview?(julien.pierre.bugs)

Updated

12 years ago
Attachment #235130 - Flags: superreview?(julien.pierre.bugs) → superreview+
(Assignee)

Comment 11

12 years ago
3.12:
/cvsroot/mozilla/security/nss/lib/pki/pki3hack.c,v  <--  pki3hack.c
new revision: 1.90; previous revision: 1.89

3.11 branch:
/cvsroot/mozilla/security/nss/lib/pki/pki3hack.c,v  <--  pki3hack.c
new revision: 1.86.28.4; previous revision: 1.86.28.3
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
Crash Signature: [@ nssArena_Destroy - nssTrustDomain_TraverseCertificatesBySubject] [@ nssArena_Destroy - nssTrustDomain_TraverseCertificatesByNickname]
You need to log in before you can comment on or make changes to this bug.