291228 - CERT_GetCertNicknames leaks memory

Status: RESOLVED DUPLICATE of bug 354313

(NSS :: Libraries, defect, P2)

Description

[Julien Pierre]

This bug was originally reported by David ([e-mail]) in the
n.p.m.crypto newsgroup.

When using a database which contains at least one user cert, and the following
program :

int main(int argc, char **argv)
{
    CERTCertNicknames *names = NULL;

    SECStatus rv ;
    
    rv = NSS_Init(".");

    if (SECSuccess != rv)
    {
        printf("Failure in NSS_Init.\n");
        exit(1);
    }
    names = CERT_GetCertNicknames(CERT_GetDefaultCertDB(), 
                                  SEC_CERT_NICKNAMES_USER, NULL);

    if (names)
    {
        CERT_FreeNicknames(names);
    }
    NSS_Shutdown();
}

Memory that's part of the list is leaked , even though CERT_FreeNicknames was
called. The leak occurs in stan code.

The report from dbx check leaks is :

Checking for memory leaks...

Actual leaks report    (actual leaks:            2  total size:         88 bytes)

  Total     Num of  Leaked     Allocation call stack
  Size      Blocks  Block
                    Address
==========  ====== =========== =======================================
        48       1 0x100106488  calloc < PR_Calloc < nss_ZAlloc <
nssCryptokiObject_Clone < get_cert_instance < STAN_GetCERTCertificateName <
CollectNicknames < nssPKIObjectCollection_Traverse <
NSSTrustDomain_TraverseCertificates < CERT_GetCertNicknames < main 
        40       1 0x1001064d8  calloc < PR_Calloc < nss_ZAlloc <
nssUTF8_Duplicate < nssCryptokiObject_Clone < get_cert_instance <
STAN_GetCERTCertificateName < CollectNicknames < nssPKIObjectCollection_Traverse
< NSSTrustDomain_TraverseCertificates < CERT_GetCertNicknames < main 
 

Possible leaks report  (possible leaks:          4  total size:       2495 bytes)

  Total     Num of  Leaked     Allocation call stack
  Size      Blocks  Block
                    Address
==========  ====== =========== =======================================
      2087       1 0x1001272b8  PR_Malloc < PL_ArenaAllocate <
nss_zalloc_arena_locked < nss_ZAlloc < nssToken_CreateFromPK11SlotInfo <
STAN_InitTokenForSlotInfo < STAN_LoadDefaultNSS3TrustDomain < nss_Init <
NSS_Init < main 
       144       1 0x100127b08  calloc < PR_Calloc < PR_NewLock <
nssToken_CreateFromPK11SlotInfo < STAN_InitTokenForSlotInfo <
STAN_LoadDefaultNSS3TrustDomain < nss_Init < NSS_Init < main 
       144       1 0x100127208  calloc < PR_Calloc < PR_NewLock <
nssArena_Create < nssToken_CreateFromPK11SlotInfo < STAN_InitTokenForSlotInfo <
STAN_LoadDefaultNSS3TrustDomain < nss_Init < NSS_Init < main 
       120       1 0x100127168  calloc < PR_Calloc < nss_ZAlloc <
nssArena_Create < nssToken_CreateFromPK11SlotInfo < STAN_InitTokenForSlotInfo <
STAN_LoadDefaultNSS3TrustDomain < nss_Init < NSS_Init < main 
 

execution completed, exit code is 1
(dbx)

Comment 1

[Julien Pierre]

Assigning to Neil.

Comment 2

[Wan-Teh Chang]

*** Bug 317975 has been marked as a duplicate of this bug. ***

Comment 3

[Wan-Teh Chang]

I didn't look at the "possible leaks" reported by dbx.
The "actual leaks" reported by dbx can be fixed by having
STAN_GetCERTCertificateName destroy 'instance'.  A naive
way to do that is as follows (See also bug 317975 comment
3):

char * 
STAN_GetCERTCertificateName(PLArenaPool *arenaOpt, NSSCertificate *c)
{
    nssCryptokiInstance *instance = get_cert_instance(c);
    char *nickname;
    nickname = STAN_GetCERTCertificateNameForInstance(arenaOpt, c, instance);
    if (instance) {
        nssCryptokiObject_Destroy(instance);
    }
    return nickname;
}

I call this fix "naive" because I didn't have time to fully
understand the code.  For example, I don't know if it's okay
to pass a NULL 'instance' to STAN_GetCERTCertificateNameForInstance.
But the above code won't crash and it will fix the leaks.

Comment 4

[Julien Pierre]

Retargetting all P2s to 3.11.3 .

Comment 5

[:Cykesiopka]

FWIW, Bug 354313 fixed the STAN_GetCERTCertificateName() leak:
  https://hg.mozilla.org/projects/nss/rev/ee5795894f78

Comment 6

[:Cykesiopka]

Should this bug stay open for the "possible leaks", or is it OK to resolve this bug as a dupe of Bug 354313? (Or something else?)

Comment 7

[:Cykesiopka]

I'm going to go ahead and just resolve this as a duplicate of Bug 354313 (see Comment 6).

Please feel free to re-open if appropriate.