[Bug 20684] New: Memory leak in CryptGenKey/CryptDestroyKey?
http://bugs.winehq.org/show_bug.cgi?id=20684 Summary: Memory leak in CryptGenKey/CryptDestroyKey? Product: Wine Version: 1.1.32 Platform: PC OS/Version: Linux Status: NEW Keywords: download, source, testcase Severity: normal Priority: P2 Component: rsaenh AssignedTo: wine-bugs(a)winehq.org ReportedBy: dank(a)kegel.com Created an attachment (id=24711) --> (http://bugs.winehq.org/attachment.cgi?id=24711) minimal test showing problem Chromium's unit_tests.exe's SignatureCreatorTest.BasicTest seems to show a leak in CryptGenKey(): 972 bytes in 1 blocks are definitely lost in loss record 836 of 921 at RtlAllocateHeap (heap.c:1423) by ??? by ??? by ??? by CryptGenKey (crypt.c:1434) by base::RSAPrivateKey::Create (rsa_private_key_win.cc:35) by SignatureCreatorTest_BasicTest_Test::TestBody (signature_creator_unittest.cc:15) I've extracted a minimal test case into the attached patch. To repeat, apply the patch and then run the rsaenh test under valgrind. It complains 972 bytes in 1 blocks are definitely lost in loss record 463 of 518 at RtlAllocateHeap (heap.c:1423) by new_object (handle.c:362) by new_key (rsaenh.c:834) by RSAENH_CPGenKey (rsaenh.c:3064) by CryptGenKey (crypt.c:1434) by func_rsaenh (rsaenh.c:2546) by run_test (test.h:535) by main (test.h:585) It seems the reference count of the generated key starts off one too high, or something? -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email Do not reply to this email, post in Bugzilla using the above URL to reply. ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=20684 --- Comment #1 from Juan Lang <juan_lang(a)yahoo.com> 2009-11-12 20:05:47 --- Shouldn't you be calling CryptReleaseContext before deleting the keyset? -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email Do not reply to this email, post in Bugzilla using the above URL to reply. ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=20684 Juan Lang <juan_lang(a)yahoo.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |RESOLVED Resolution| |INVALID --- Comment #2 from Juan Lang <juan_lang(a)yahoo.com> 2009-11-13 10:37:21 --- Let me rephrase that not to be rhetorical: you should be calling CryptReleaseContext. That's the source of the leak. -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email Do not reply to this email, post in Bugzilla using the above URL to reply. ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=20684 Dan Kegel <dank(a)kegel.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |REOPENED Resolution|INVALID | --- Comment #3 from Dan Kegel <dank(a)kegel.com> 2009-11-13 12:51:32 --- Adding that to the little test case gets rid of the leak there, but the real code already has the proper number of calls to CryptReleaseContext, and leaks anyway, so maybe something more subtle is going on. To repeat: $ wget http://kegel.com/wine/chromium/chromium-tests.tar.bz2 $ tar -xjvf chromium-tests.tar.bz2 $ valgrind --leak-check=full --trace-children=yes wine src/chrome/Debug/base_unittests --gtest_filter=SignatureCreatorTest.* I added log statements to show where the keys and context were acquired and released, and will attach log files. -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email Do not reply to this email, post in Bugzilla using the above URL to reply. ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=20684 Dan Kegel <dank(a)kegel.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #24711|0 |1 is obsolete| | --- Comment #4 from Dan Kegel <dank(a)kegel.com> 2009-11-13 12:53:46 --- Created an attachment (id=24720) --> (http://bugs.winehq.org/attachment.cgi?id=24720) Output of valgrind --trace-children=yes --leak-check=full wine base_unittests.exe --gtest_filter=SignatureCreatorTest.* -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email Do not reply to this email, post in Bugzilla using the above URL to reply. ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=20684 --- Comment #5 from Dan Kegel <dank(a)kegel.com> 2009-11-13 12:56:06 --- Created an attachment (id=24721) --> (http://bugs.winehq.org/attachment.cgi?id=24721) rzipped log of WINEDEBUG=+crypt,+handle,warn+heap,+relay wine base_unittests.exe --gtest_filter=SignatureCreatorTest.* -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email Do not reply to this email, post in Bugzilla using the above URL to reply. ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=20684 --- Comment #6 from Juan Lang <juan_lang(a)yahoo.com> 2009-11-19 19:59:48 --- Color me skeptical. You didn't catch the error in the minimal test case, so I'm not certain you will have done so in the large test suite. Static analysis might be a better tool here. -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email Do not reply to this email, post in Bugzilla using the above URL to reply. ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=20684 Juan Lang <juan_lang(a)yahoo.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|REOPENED |RESOLVED Resolution| |INVALID --- Comment #7 from Juan Lang <juan_lang(a)yahoo.com> 2009-11-19 20:53:05 --- In fact, I don't think this is an appropriate use of bugzilla. All you can say is that something is leaking, but you can't be certain whether it's your test suite, the chromium code, or rsaenh. A bug should show a difference in behavior with Windows, and so far this doesn't. Feel free to reopen if you can narrow it down to a (small) test that you're certain shouldn't leak, and we can investigate that one. -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email Do not reply to this email, post in Bugzilla using the above URL to reply. ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=20684 Dmitry Timoshkov <dmitry(a)codeweavers.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |CLOSED --- Comment #8 from Dmitry Timoshkov <dmitry(a)codeweavers.com> 2009-11-23 06:33:40 --- Closing invalid. -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email Do not reply to this email, post in Bugzilla using the above URL to reply. ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=20684 Juan Lang <juan_lang(a)yahoo.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|CLOSED |REOPENED Resolution|INVALID | --- Comment #9 from Juan Lang <juan_lang(a)yahoo.com> 2009-12-07 16:07:00 --- I'm going to reopen this now that Dan's valgrind logs include leaks for the Wine test suite, and rsaenh shows leaks. E.g. here's the latest log: http://kegel.com/wine/valgrind/logs/2009-12-07-08.39/vg-rsaenh_rsaenh.txt It includes leaks such as: 972 bytes in 1 blocks are definitely lost at notify_alloc (heap.c:247) by RtlAllocateHeap (heap.c:1697) by ??? by ??? by ??? by ??? by ??? by ??? by ??? by CryptAcquireContextW (crypt.c:505) by CryptAcquireContextA (crypt.c:573) by test_key_initialization (rsaenh.c:2528) by func_rsaenh (rsaenh.c:2564) by run_test (test.h:535) by main (test.h:585) The referenced line of test_key_initialization is included in this body: result = CryptAcquireContext(&prov2, szContainer, szProvider, PROV_RSA_FULL, 0); ok(result, "%08x\n", GetLastError()); result = CryptGetUserKey(prov2, AT_KEYEXCHANGE, &hKey); ok(result, "%08x\n", GetLastError()); if (result) CryptDestroyKey(hKey); CryptReleaseContext(prov2, 0); It appears to me as though the reference counting is valid here, so the report of a leak seems accurate. -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email Do not reply to this email, post in Bugzilla using the above URL to reply. ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=20684 --- Comment #10 from Juan Lang <juan_lang(a)yahoo.com> 2009-12-09 14:14:59 --- Commit 035c323054c23c83203b7ac9b8d9bc7a050336ba improved one leak (in CPSignHash), and commit d69b00d630200ce013a3a8a97eea987e1165e89a removed some leaks in the tests, see the diffs in the subsequent log: http://kegel.com/wine/valgrind/logs/2009-12-07-08.39/diff-rsaenh_rsaenh.txt (Ignore all the reads of uninitialized values, those were likely the result of a slightly broken build.) And commits 71880e48184ec66fbb04d074404c5c3e8f32fe35 and 71b4ac9c71dfa7736a7e7cff09d5899c209fd7fe seem to have helped too, see the diffs in today's log: http://kegel.com/wine/valgrind/logs/2009-12-09-09.00/diff-rsaenh_rsaenh.txt Still, a few seem to remain: http://kegel.com/wine/valgrind/logs/2009-12-09-09.00/vg-rsaenh_rsaenh.txt -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email Do not reply to this email, post in Bugzilla using the above URL to reply. ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=20684 --- Comment #11 from Juan Lang <juan_lang(a)yahoo.com> 2009-12-09 16:45:43 --- I sent a patch which I hope will fix the remaining leaks: http://www.winehq.org/pipermail/wine-patches/2009-December/082514.html -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email Do not reply to this email, post in Bugzilla using the above URL to reply. ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=20684 --- Comment #12 from Juan Lang <juan_lang(a)yahoo.com> 2009-12-10 11:49:28 --- Patch was committed (f9a475c808b3bbc4980f3225dfa2a7397df9f4db), awaiting new results with baited breath :) -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email Do not reply to this email, post in Bugzilla using the above URL to reply. ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=20684 Juan Lang <juan_lang(a)yahoo.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|REOPENED |RESOLVED Resolution| |FIXED --- Comment #13 from Juan Lang <juan_lang(a)yahoo.com> 2009-12-10 22:53:57 --- This appears to be fixed. Today's valgrind logs (http://kegel.com/wine/valgrind/logs/2009-12-10-17.26/) show no errors in the rsaenh tests, and the majority of the crypt32 errors have disappeared too. -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email Do not reply to this email, post in Bugzilla using the above URL to reply. ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=20684 Alexandre Julliard <julliard(a)winehq.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |CLOSED --- Comment #14 from Alexandre Julliard <julliard(a)winehq.org> 2009-12-18 13:07:53 --- Closing bugs fixed in 1.1.35. -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email Do not reply to this email, post in Bugzilla using the above URL to reply. ------- You are receiving this mail because: ------- You are watching all bug changes.
participants (1)
-
wine-bugs＠winehq.org