403888 - memory leak in trustdomain.c

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P2)

Description

[Boying Lu]

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())

Comment 1

[Boying Lu]

Attachment: patch

Comment 2

[Wan-Teh Chang]

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.

Comment 3

[Boying Lu]

Attachment: new patch

Thank Wan, I modified the patch and tested it. I've verified that the return code 
is SECSuccess

Comment 4

[Wan-Teh Chang]

Attachment: 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.

Comment 7

[Julien Pierre]

We also want this fix on the NSS_3_11_BRANCH .

Comment 8

[Julien Pierre]

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

Asking for 2nd review for branch.

Comment 9

[Nelson Bolyard (seldom reads bugmail)]

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)

Comment 10

[Julien Pierre]

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