Created attachment 344238 [details] test case Description of problem: While investigating huge memory leaks in curl I discovered the function pem_DestroyInternalObject() is pretty broken, moreover it wasn't called at all. Attached patch frees internal objects on module destruction. It gives zero memory leaks on attached (brute force) test case, but it is still leaking within pfind.c in more complex cases. As time permits other patches will follow. Version-Release number of selected component (if applicable): nss-3.12.2-5.fc10 Steps to Reproduce: 1. run the attached test case Actual results: huge memory leaks Additional info: patch ready and awaiting review
Created attachment 344239 [details] proposed fix
Created attachment 344296 [details] follow-up patch An incremental patch is attached. It catches tons of memory leaks while using a client certificate.
(In reply to comment #2) Kamil, Reassigning to you, please check the ? review flag and reassign back to me.
Oops, I messed it up again. I should have asked you to keep it assigned to you and set the ? flag with me as the reviewer. Now it came out assigned to me with you as the reviewer :-) Sorry about that.
Both the proposed fix and follow up patch get +r from me.
First patch +r In the second patch, you are adding a nss_ZFreeIf() before each allocation of the key data. Is this really needed? I don't think there is a way to re-load a key on top of an existing one so these should always be unallocated.
It is, because the memory is really freed by the nss_ZFreeIf() calls. I don't know the PEM reader internals enough to explain why. It might be another bug. But if you omit them, you will loose the previously allocated memory on the second allocation. I have nothing like minimal example for this one, but I used this (you need to replace userID with the your one): LD_PRELOAD=libnsspem.so valgrind --leak-check=full curl 'https://koji.fedoraproject.org/koji/userinfo?userID=750' --verbose --silent --cert $HOME/.fedora.cert -o /dev/null
(In reply to comment #7) > this (you need to replace userID with the your one): Actually you don't need to change anything. Feel free to download my build list :-)
Spotted another memory leak in prsa.c. pem_PopulateModulusExponent allocates an arena, then if the test commented by /* make sure we have the right objects */ fails it returns without deallocating the arena.
Created attachment 345202 [details] proposed fix for possibly undestroyed arena in prsa.c Good catch. It's possible memory leak for nothing. Moving the allocation of arena down should fix it.
nss-3.12.3.99.3-2.11.3.fc11 has been submitted as an update for Fedora 11. http://admin.fedoraproject.org/updates/nss-3.12.3.99.3-2.11.3.fc11
nss-3.12.3.99.3-2.11.3.fc11 has been pushed to the Fedora 11 testing repository. If problems still persist, please make note of it in this bug report. If you want to test the update, you can install it with su -c 'yum --enablerepo=updates-testing update nss'. You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F11/FEDORA-2009-6948
nss-3.12.3.99.3-2.10.4.fc10 has been pushed to the Fedora 10 testing repository. If problems still persist, please make note of it in this bug report. If you want to test the update, you can install it with su -c 'yum --enablerepo=updates-testing update nss'. You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F10/FEDORA-2009-7017
nss-3.12.3.99.3-2.11.3.fc11 has been pushed to the Fedora 11 stable repository. If problems still persist, please make note of it in this bug report.
nss-3.12.3.99.3-2.10.4.fc10 has been pushed to the Fedora 10 stable repository. If problems still persist, please make note of it in this bug report.