404919 - memory leak in sftkdb_ReadSecmodDB() (sftkmod.c)

Status: RESOLVED FIXED

(NSS :: Libraries, defect)

Description

[Boying Lu]

Using solaris memory leak detection tool libumem.so, I found a memory leak in sftkdb_ReadSecmodDB().  Here is the related output of libumem.so:
                 libumem.so.1`umem_cache_alloc_debug+0x14f
                 libumem.so.1`umem_cache_alloc+0x144
                 libumem.so.1`umem_alloc+0xc5
                 libumem.so.1`malloc+0x27
                 libnspr4.so`PR_Malloc+0x5d
                 libnspr4.so`GrowStuff+0x95
                 libnspr4.so`fill2+0xb7
                 libnspr4.so`cvt_s+0xc8
                 libnspr4.so`dosprintf+0xcdf
                 libnspr4.so`PR_vsmprintf+0x50
                 libnspr4.so`PR_smprintf+0x34
                 0xf8a1ae08
                 0xf8a1b48e
                 0xf89fe3c5
                 libnss3.so`SECMOD_GetModuleSpecList+0x42
                 libnss3.so`SECMOD_LoadModule+0x156
                 libnss3.so`nss_Init+0x270
                 libnss3.so`NSS_NoDB_Init+0x57
                 unsigned nsNSSComponent::InitializeNSS+0x18e
                 unsigned nsNSSComponent::Init+0x15a
                 unsigned nsNSSComponentConstructor+0x9f
                 libxpcom_core.so`unsigned nsGenericFactory::CreateInstance+0x46
                 ...
Test Case:
1. firefox -P
2. click "Exit" button

Comment 1

[Boying Lu]

Attachment: patch

Comment 2

[Wan-Teh Chang]

Comment on attachment 289785 [details] [diff] [review]
patch

We also need to free olddbname at the other
exit points of the if block.

We can move the "bail" label into the if block,
and free olddbname there, like this:

bail:
        if (olddbname) {
            PR_smprintf_free(olddbname);
        }
    }

Comment 3

[Boying Lu]

Attachment: mv label "bail" into the "if" block

Thanks for the comment. I re-made the patch

Comment 4

[Wan-Teh Chang]

Comment on attachment 289875 [details] [diff] [review]
mv label "bail" into the "if" block

r=wtc.

I will fix the indentation when I check in
the patch.  In NSS, we indent by four spaces.

Comment 5

[Wan-Teh Chang]

Attachment: mv label "bail" into the "if" block (as checked in)

I checked in this patch on the NSS trunk for NSS 3.12.

Checking in sftkmod.c;
/cvsroot/mozilla/security/nss/lib/softoken/sftkmod.c,v  <--  sftkmod.c
new revision: 1.2; previous revision: 1.1
done