memory leak in sftkdb_ReadSecmodDB() (sftkmod.c)

RESOLVED FIXED in 3.12

Status

NSS
Libraries
RESOLVED FIXED
10 years ago
9 years ago

People

(Reporter: Boying Lu, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

10 years ago
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
(Reporter)

Comment 1

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

Comment 2

10 years ago
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);
        }
    }
(Reporter)

Updated

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

Comment 3

10 years ago
Created attachment 289875 [details] [diff] [review]
mv label "bail" into the "if" block

Thanks for the comment. I re-made the patch
Attachment #289875 - Flags: review?(wtc)

Comment 4

10 years ago
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.
Attachment #289875 - Flags: review?(wtc) → review+

Comment 5

10 years ago
Created attachment 290488 [details] [diff] [review]
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
Attachment #289875 - Attachment is obsolete: true

Updated

10 years ago
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → 3.12

Updated

9 years ago
Duplicate of this bug: 401072
You need to log in before you can comment on or make changes to this bug.