Two things: 1. It calls SECKEY_DestroyPrivateKeyInfo with freeit==PR_TRUE. This will free the arena...but then we turn around and free the arena a second time. 2. It doesn't set the arena field of the PrivateKeyInfo, so DestroyPrivateKeyInfo will still try to call free() on the individual secitems in the PKI. This doesn't fail immediately, but causes the memory allocator to become unstable and fail later--Yuck!
Created attachment 85828 [details] [diff] [review] new and improved This patch is a little cleaner.
Wan-Teh: is it ok to check this in?
Comment on attachment 85828 [details] [diff] [review] new and improved r=wtc. > temparena = PORT_NewArena(DER_DEFAULT_CHUNKSIZE); >- pki = PORT_ZNew(SECKEYPrivateKeyInfo); >+ pki = PORT_ArenaZAlloc(SECKEYPrivateKeyInfo); >+ pki->arena = temparena; >+ temparena = NULL; The statement "temparena = NULL;" is not necessary.
Agreed. I put it there as more of a reminder that temparena should not be freed. But I suppose it clutters up the code so I'll take it out.
Created attachment 85851 [details] [diff] [review] ArenaZAlloc -> ArenaZNew The last patch was broken. It called ArenaZAlloc instead of ArenaZNew, and didn't pass in the arena. The compiler found this problem.
Checked in to NSS_3_4_BRANCH (for 3.4.2) and trunk (for 3.6 and beyond). /cvsroot/mozilla/security/nss/lib/pk11wrap/pk11pk12.c,v <-- pk11pk12.c new revision: 18.104.22.168; previous revision: 1.5 /cvsroot/mozilla/security/nss/lib/pk11wrap/pk11pk12.c,v <-- pk11pk12.c new revision: 1.6; previous revision: 1.5
Comment on attachment 85851 [details] [diff] [review] ArenaZAlloc -> ArenaZNew r=wtc. Please also check it into the NSS_3_5_BRANCH. Thanks.