Closed Bug 447845 Opened 16 years ago Closed 14 years ago

ldapssl_client_init leaks memory in failure case

Categories

(Directory :: LDAP C SDK, defect)

Product:
Directory
Component:
LDAP C SDK
Platform:
Other
Other
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED DUPLICATE of bug 485732

People

(Reporter: egyfan, Assigned: mcs)

References

Details

User-Agent:       Mozilla/4.0 (compatible; MSIE 7.0; Windows NT 6.0; SLCC1; .NET CLR 2.0.50727; .NET CLR 3.0.04506; .NET CLR 1.0.3705; .NET CLR 1.1.4322; InfoPath.2)
Build Identifier: ldapcsdk 605

If I call ldapssl_client_init like following:

int nErrorCode = ldapssl_client_init("/cert_dir",NULL);

where there is no actual cert/key db under /cert_dir, then I see memory leak. wdb reports 232 bytes leak.

Reproducible: Always

Steps to Reproduce:
1.
2.
3.



// test program

#include <stdio.h>
#include <ldap.h>
#include <ldap_ssl.h>

int
main(int argc, char *argv[])
{
    int nErrorCode = ldapssl_client_init("/home/efan/cert_dir",NULL);

    if (nErrorCode) {
        fprintf(stderr, "Error %d returned from ldapssl_client_init\n", nErrorCode);
        return nErrorCode;
    } else {
        fprintf(stdout, "Succeed.\n");
        return 0;
    }
}
Sorry for the delayed response (I lost this bug report in my Inbox).

There may be some "one time" leaks in the NSS code.  Can you tell us where the leaked memory was allocated?

Cc: Rich Megginson who spends a lot more time working on the LDAP C SDK code than I do.
What tool are you using to detect the memory leak?  Can you provide a stack trace of the leak?
I used WDB on HP-UX. I believe this is onetime leak. But our customer's application call this routine multiple times, so they don't like this leak.

BTW, I already left the team. So I will pass this bug to the right person in the team and let him handle any further communications.

Thanks,
Emily.
This seems like a NSS bug, they're allocating something and then abandoning pointers to it when they can't find cert/key db files.  There is nothing returned from NSS_Initialize() that we could free ourselves.  I would avoid having LDAP SDK verify whether cert8.db and key3.db exist since we could link with a new NSS that might use new file names.

240 bytes leaked at 0x60000000010cdd90 (76.92% of all bytes leaked)
#0  PR_Calloc() from libnspr4.so
#1  PR_NewMonitor() from libnspr4.so
#2  nsslowcert_OpenCertDB() from libsoftokn3.so
#3  sftk_OpenCertDB() from libsoftokn3.so
#4  sftk_DBInit() from libsoftokn3.so
#5  SFTK_SlotReInit() from libsoftokn3.so
#6  SFTK_SlotInit() from libsoftokn3.so
#7  nsc_CommonInitialize() from libsoftokn3.so
#8  NSC_Initialize() from libsoftokn3.so
#9  secmod_ModuleInit() from libnss3.so
#10 SECMOD_LoadPKCS11Module() from libnss3.so
#11 SECMOD_LoadModule() from libnss3.so
#12 SECMOD_LoadModule() from libnss3.so
#13 nss_Init() from libnss3.so
#14 NSS_Initialize() from libnss3.so
#15 ldapssl_basic_init() from libssldap60.so
#16 ldapssl_clientauth_init() from libssldap60.so
#17 ldapssl_client_init() from libssldap60.so
#18 main() at certtest.c:10
#19 main_opd_entry() from /usr/lib/hpux64/dld.so

-------------------------------------------------------------------------
72 bytes leaked at 0x60000000010bcad0 (23.08% of all bytes leaked)
#0  PR_Calloc() from libnspr4.so
#1  PR_NewMonitor() from libnspr4.so
#2  nsslowcert_OpenCertDB() from libsoftokn3.so
#3  sftk_OpenCertDB() from libsoftokn3.so
#4  sftk_DBInit() from libsoftokn3.so
#5  SFTK_SlotReInit() from libsoftokn3.so
#6  SFTK_SlotInit() from libsoftokn3.so
#7  nsc_CommonInitialize() from libsoftokn3.so
#8  NSC_Initialize() from libsoftokn3.so
#9  secmod_ModuleInit() from libnss3.so
#10 SECMOD_LoadPKCS11Module() from libnss3.so
#11 SECMOD_LoadModule() from libnss3.so
#12 SECMOD_LoadModule() from libnss3.so
#13 nss_Init() from libnss3.so
#14 NSS_Initialize() from libnss3.so
#15 ldapssl_basic_init() from libssldap60.so
#16 ldapssl_clientauth_init() from libssldap60.so
#17 ldapssl_client_init() from libssldap60.so
#18 main() at certtest.c:10
#19 main_opd_entry() from /usr/lib/hpux64/dld.so
Guys and Gals, Does LDAP C SDK have a shutdown function? 
If not, what code is expected to release the memory allocated by 
ldapssl_client_init ?

NSS has a shutdown function that releases all that stuff.  So does NSPR.
If you don't call the Shutdown functions, lots of memory will be leaked.
You need to find some place to call them.  You COULD ask the application 
to do that, or, if LDAP C SDK has shutdown functions, those would be logical
places to do it.
Thanks Nelson.  It seems we register a shutdown handler, but in this case the application programmer is not shutting LDAP C SDK down.  They want NSS_Initialize() called again with a new certificate database path.  Can we call NSS_Shutdown() to clean up if NSS_Initialize() doesn't return SECSuccess?
What version of NSS is being used?  From the stack, I can see that it must 
be older than 3.12.  What is it?

> Can we call NSS_Shutdown() to clean up if NSS_Initialize() doesn't return 
> SECSuccess?

Is that the situation here?  Comment 0 says nothing about NSS_Init failing.
Of this leak occurs when NSS_Init fails, then I agree it's an NSS bug.

> the application programmer is not shutting LDAP C SDK down.  They want 
> NSS_Initialize() called again with a new certificate database path. 

I'm familiar with a certain company who wants to be able to substitute a 
new client cert and client private key into a running LDAP client program.
There have been many posts about that in various Mozilla lists/groups.
I see no evidence in this bug that this bug is related to that issue. 
I'd be happy to discuss that issue (again) in the appropriate venue, 
but this bug (about a memory leak) isn't the right venue.  There are 
numerous solutions available to that customer, but NONE of them is to 
switch cert and key DBs in a running process.  So, name the right venue
for that topic and we'll discuss it there.
(In reply to comment #7)
> What version of NSS is being used?  From the stack, I can see that it must 
> be older than 3.12.  What is it?

3.11.7.

> Is that the situation here?  Comment 0 says nothing about NSS_Init failing.
> Of this leak occurs when NSS_Init fails, then I agree it's an NSS bug.

As comment 0 mentions, she's passing a path that doesn't contain a cert or key database to ldapssl_client_init().  This results in NSS_Initialize() failing and then my heap analyzer claims NSS abandons two allocations.

> I'm familiar with a certain company who wants to be able to substitute a 
> new client cert and client private key into a running LDAP client program.

That is unrelated.  In this case, I suppose they are attempting to initialize NSS with different paths until they find one that has cert and key databases and the call succeeds.
I'm pretty sure this memory will be cleaned up if you run NSS_Shutdown when you shutdown.  Are you sure the pointers are being discarded, or are they being cached?
NSS_Shutdown() doesn't seem to release the memory and the pointers appear to get discarded unless the memory is referenced by another method, such as offsets from elsewhere or something else that confuses the leak detectors.  I've reproduced on Linux with Valgrind using this quick test:

int i = NSS_Initialize("/tmp", "nonexistent-", "nonexistent-","secmod.db", NSS_INIT_READONLY);
NSS_Shutdown();
Did you guys look at bug 485732 ?
(In reply to comment #11)
> Did you guys look at bug 485732 ?

Thanks Nelson. So should we close this bug as a dup of 485732?  Or is there still some LDAP C SDK issue to solve here?
Status: UNCONFIRMED → RESOLVED
Closed: 14 years ago
Resolution: --- → DUPLICATE
You need to log in before you can comment on or make changes to this bug.