Call CMMFCertRepContent * c = CMMF_CreateCertRepContentFromDER(CERT_GetDefaultCertDB(), data, len); if (c) CMMF_DestroyCertRepContent(c); After this sequence, shutting down and reinitializing NSS fails. Always reproducible.
Julien, could you take a look at this bug? Thanks.
Kai, It would be useful if you had some sample data to feed to this function that reproduced the leak, or some programmatic way to generate that data. I took a look at the code and I have a suspicion of where the leak happens. I believe it is in crmf/asn1cmf.c, in cmmf_DecodeDERCertificate , in the CERT_NewTempCertificate call. I don't see where that cert is getting freed.
Julien, I changed PSM to dump the argument passed on to CMMF_CreateCertRepContentFromDER to a file. I have sent you personal mail with such a file attached, that was created using the procdure explained in bug 189974. Does this help?
Kai, Thanks for the file. I created a test program that called this function to create the context, and then immediately destroyed it. I confirmed using Solaris dbx showleaks that there is a leak. It is in fact occurring in the function I mentioned in comment #2. I will create a patch shortly.
Created attachment 112937 [details] [diff] [review] Fix memory leak - Correct erroneous test for isDecoded . Because of this mistake, all the cert freeing code was dead code - remove the erroneous freeing of caPubs . This was allocated in an arena through the ASN.1 decoder and is freed by the PORT_FreeArena call
Thanks for the patch, Julien. I just tested it, and it appears to work. I can confirm, when applying both patches from this and bug 189974, profile switching does work correctly when using the testcase described in bug 189974. While I think this is not required for Mozilla 1.3 beta, it would be great if this fix could be included in the version of NSS that will be supplied for Mozilla 1.3 final.
Let's target this for NSS_3_7_BRANCH (3.7.2). Please get two code reviews. Thank you for tracking down this memory leak, Julien.
Created attachment 113046 [details] [diff] [review] better patch The check for isDecoded was actually unnecessary, since the memory is Zallocated . So we always try to free, even if decoding failed (potentially partially).
Comment on attachment 113046 [details] [diff] [review] better patch r=wtc. Nelson, could you review this patch too?
Checked in to NSS 3.8 (tip) . Checking in respcmn.c; /cvsroot/mozilla/security/nss/lib/crmf/respcmn.c,v <-- respcmn.c new revision: 1.7; previous revision: 1.6 done Still waiting for Nelson's review and tinderbox results for check-in into 3.7 branch.
Thanks. Checked in to 3.7 branch as well. cvs commit: Examining . Checking in respcmn.c; /cvsroot/mozilla/security/nss/lib/crmf/respcmn.c,v <-- respcmn.c new revision: 188.8.131.52; previous revision: 1.6 done
I confirm the second patch fixes the problem, too.