PK11_ListCerts calls STAN_GetCERTCertificateName to allocate the appData fields in the cert list nodes it returns. STAN_GetCERTCertificateName returns a nickname whose memory is allocated with PORT_Alloc. So we need to free the cert list nodes' appData with PORT_Free. CERT_DestroyCertList isn't doing that. This is a memory leak. Three possible solutions. 1. Document that the caller of PK11_ListCerts is responsible for freeing the cert list nodes' appData with PORT_Free before calling CERT_DestroyCertList. 2. Modify STAN_GetCERTCertificateName so that it uses the cert list's arena to allocate the nickname. 3. Add an appDataDestructor function pointer to the CERTCertListStr structure, and modify CERT_DestroyCertList to invoke appDataDestructor on the cert list nodes' appData.
Created attachment 131211 [details] [diff] [review] Proposed patch This patch implements solution 2: allocate the nicknames from the cert list's arena. It improves upon Julien's work. Note that for clarity this patch is against the tip of NSS *before* Julien's patch (rev. 1.119 of pk11cert.c).
Comment on attachment 131211 [details] [diff] [review] Proposed patch Julien, Ian, please review this patch. Questions for Ian: 1. Is it okay for STAN_GetCERTCertificateName to take PRArenaPool (a non-Stan data type) as an argument? I think it is okay because STAN_GetCERTCertificateName is not a Stan function. 2. Is 'arenaOpt' your naming convention for an arena argument that may be NULL, which means allocating from the heap instead?
Comment on attachment 131211 [details] [diff] [review] Proposed patch This looks fine. However, you'll need to undo my other patch (which frees from the global heap and reallocates into the arena) before you can apply this to the tip and check it in.
Attachment #131211 - Flags: review?(jpierre) → review+
Created attachment 131355 [details] [diff] [review] Proposed patch v1.1 The only change in this patch is that I use the more correct name PLArenaPool rather than PRArenaPool.
Thanks for the review, Julien. Yes, one needs to undo your other patch before applying my patch. I've checked in the patch on the NSS trunk (NSS 3.9) and NSS_3_8_BRANCH (NSS 3.8.2).
Status: NEW → RESOLVED
Last Resolved: 15 years ago
Resolution: --- → FIXED
Target Milestone: --- → 3.9
You need to log in before you can comment on or make changes to this bug.