memory leak in trustdomain.c

RESOLVED FIXED in 3.11.10

Status

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

People

(Reporter: Boying Lu, Assigned: Julien Pierre)

Tracking

({memory-leak})

unspecified
3.11.10
memory-leak

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

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

Description

11 years ago
Using memory leak detection tool libumem.so privided by opensolaris, I found there is a memory leak in trustdomain.c.

Following are steps I used:
1. ./firefox -P
2. close profile manager

libumem.so reports following memory leaks:
                 libumem.so.1`umem_cache_alloc_debug+0x14f
                 libumem.so.1`umem_cache_alloc+0x180
                 libumem.so.1`umem_alloc+0xc5
                 libumem.so.1`malloc+0x27
                 libumem.so.1`calloc+0x47
                 libnspr4.so`PR_Calloc+0x65
                 libnss3.so`PORT_ZAlloc+0x46
                 libnss3.so`ocsp_InitStatusChecking+0x83
                 libnss3.so`CERT_EnableOCSPChecking+0x64
                 libpipnss.so`void setOCSPOptions+0x99
                 libpipnss.so`unsigned nsNSSComponent::InitializeNSS+0x83d
                 libpipnss.so`unsigned nsNSSComponent::Init+0x18a
                 libpipnss.so`unsigned nsNSSComponentConstructor+0xb7
                 libxpcom_core.so`unsigned nsGenericFactory::CreateInstance+0x46
                 
                 libxpcom_core.so`unsigned nsComponentManagerImpl::CreateInstanc
                 eByContractID+0xd6
                 
                 libxpcom_core.so`unsigned nsComponentManagerImpl::GetServiceByC
                 ontractID+0x1c8      
                 libxpcom_core.so`unsigned CallGetService+0x71
                 
                 libxpcom_core.so`unsigned nsGetServiceByContractIDWithError::op
                 eratorconst+0x35
                 
                 libxpcom_core.so`void nsCOMPtr_base::assign_from_gs_contractid_
                 with_error+0x32
                 ...

After some investigation, I found the root cause is 
   g_default_trust_domain->statusConfig and 
   g_default_trust_domain->statusConfig->statusContext
are created (malloc'ed) but not freed in NSSTrustDomain_Destroy (called by STAN_Shutdown())
(Reporter)

Updated

11 years ago
Component: Security → General
QA Contact: firefox → general
(Reporter)

Comment 1

11 years ago
Created attachment 288832 [details] [diff] [review]
patch
Attachment #288832 - Flags: review?(wtc)

Updated

11 years ago
Keywords: footprint → mlk

Comment 2

11 years ago
Boying, thanks for the patch.

We should free td->statusConfig using the td->statusConfig->statusDestroy
function pointer, like this:

+#ifdef NSS_3_4_CODE
+        if (td->statusConfig) {
+           td->statusConfig->statusDestroy(td->statusConfig);
+           td->statusConfig = NULL;
+        }
+#endif

But we're not calling td->statusConfig->statusDestroy right now.
So please carefully test it (single step in the debugger) and
make sure it returns SECSuccess.
Assignee: nobody → nobody
Component: General → Libraries
Product: Firefox → NSS
QA Contact: general → libraries
Target Milestone: --- → 3.12
Version: Trunk → unspecified
(Reporter)

Updated

11 years ago
Attachment #288832 - Attachment is obsolete: true
Attachment #288832 - Flags: review?(wtc)
(Reporter)

Comment 3

11 years ago
Created attachment 288969 [details] [diff] [review]
new patch

Thank Wan, I modified the patch and tested it. I've verified that the return code 
is SECSuccess
Attachment #288969 - Flags: review?(wtc)

Comment 4

11 years ago
Created attachment 289023 [details] [diff] [review]
new patch (as checked in)

Boying, thanks for the new patch.  I checked it in on the NSS trunk (NSS 3.12).

Checking in trustdomain.c;
/cvsroot/mozilla/security/nss/lib/pki/trustdomain.c,v  <--  trustdomain.c
new revision: 1.56; previous revision: 1.55
done

I made some minor formatting changes and removed the NSS_3_4_CODE
macro, which just became obsolete.
Attachment #288969 - Attachment is obsolete: true
Attachment #288969 - Flags: review?(wtc)

Updated

11 years ago
Status: NEW → RESOLVED
Last Resolved: 11 years ago
OS: OpenSolaris → All
Hardware: PC → All
Resolution: --- → FIXED
(Assignee)

Updated

11 years ago
Duplicate of this bug: 391773
(Assignee)

Updated

11 years ago
Duplicate of this bug: 393181
(Assignee)

Updated

11 years ago
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 7

11 years ago
We also want this fix on the NSS_3_11_BRANCH .
Assignee: nobody → julien.pierre.boogz
Status: REOPENED → NEW
Priority: -- → P2
Target Milestone: 3.12 → 3.11.10
(Assignee)

Comment 8

11 years ago
Comment on attachment 289023 [details] [diff] [review]
new patch (as checked in)

Asking for 2nd review for branch.
Attachment #289023 - Flags: review?(nelson)
Comment on attachment 289023 [details] [diff] [review]
new patch (as checked in)

I guess Wan-Teh implicitly reviewed the patch for the trunk.  
r=nelson for the branch, assuming the patch applies cleanly there. (I expect it will)
Attachment #289023 - Flags: review?(nelson) → review+
(Assignee)

Comment 10

11 years ago
Thanks, Nelson. The patch applied cleanly to NSS_3_11_BRANCH. I checked it in.

Checking in trustdomain.c;
/cvsroot/mozilla/security/nss/lib/pki/trustdomain.c,v  <--  trustdomain.c
new revision: 1.51.28.5; previous revision: 1.51.28.4
done
Status: NEW → RESOLVED
Last Resolved: 11 years ago11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.