[Bug 21568] New: advapi32/crypt tests crash with +heap
http://bugs.winehq.org/show_bug.cgi?id=21568 Summary: advapi32/crypt tests crash with +heap Product: Wine Version: 1.1.37 Platform: x86-64 URL: http://test.winehq.org/data/7aaaf738ecd06c12bfd69200ee 74abd5cc9aef8b/wine_ae-ub910-heap/advapi32:crypt.html OS/Version: Linux Status: NEW Keywords: download, source, testcase Severity: minor Priority: P2 Component: advapi32 AssignedTo: wine-bugs(a)winehq.org ReportedBy: austinenglish(a)gmail.com Created an attachment (id=26007) --> (http://bugs.winehq.org/attachment.cgi?id=26007) terminal output =>0 0x7ec1ac0d CryptSetKeyParam+0x98(hKey=1334760, dwParam=4, pbData="", dwFlags=4) [/home/austin/wine-git/dlls/advapi32/crypt.c:1961] in advapi32 (0x0063fcf8) ... 0x7ec1ac0d CryptSetKeyParam+0x98 [/home/austin/wine-git/dlls/advapi32/crypt.c:1961] in advapi32: movl 0x0(%eax),%eax 1961 if (!key || !pbData || !key->pProvider || key->pProvider->dwMagic != MAGIC_CRYPTPROV) -- 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=21568 --- Comment #1 from Juan Lang <juan_lang(a)yahoo.com> 2010-02-01 14:11:41 --- It's hard to diagnose this, because your backtrace doesn't have debug symbols for the test files. Still, I'll take a guess that it's this one, in test_incorrect_api_usage(): result = pCryptDestroyKey(hKey2); ok (result, "%d\n", GetLastError()); dwTemp = CRYPT_MODE_ECB; result = pCryptSetKeyParam(hKey2, KP_MODE, (BYTE*)&dwTemp, sizeof(DWORD)); ok (!result && GetLastError() == ERROR_INVALID_PARAMETER, "%d\n", GetLastError()); That is, a destroyed key is than used. The bad address is 0xfeeefeee, which corresponds to the free filler (in dlls/ntdll/heap.c): #define ARENA_FREE_FILLER 0xfeeefeee Basically, I think this is a false positive. An app was doing something bogus in the first place, and we wrote a test that mimics its behavior. I'm not sure how to fix the tests to pass in this case. -- 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=21568 Semen <sabsem(a)yandex.ru> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |sabsem(a)yandex.ru -- 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=21568 Austin Lund <austin.lund(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |austin.lund(a)gmail.com --- Comment #2 from Austin Lund <austin.lund(a)gmail.com> 2010-07-09 03:54:08 --- Does this really need to be tested for? MSDN does say that if you call CryptDestroy you should not use the handle again. Is there an application which uses this functionality? If so, then it's wine that needs fixing and not the tests. Is there a way to know if a pointer has been freed or not? -- 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=21568 --- Comment #3 from Juan Lang <juan_lang(a)yahoo.com> 2010-07-09 11:57:13 --- (In reply to comment #2)
Does this really need to be tested for? MSDN does say that if you call CryptDestroy you should not use the handle again. Is there an application which uses this functionality?
Yes. The test code I already pointed out says which app. -- 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=21568 --- Comment #4 from Austin Lund <austin.lund(a)gmail.com> 2010-07-09 22:46:40 --- OK. So wine needs to be fixed. Is there a way to know if a pointer has been freed? Just looking through MSDN it seems there isn't. If that's the case then a list of valid handles will need to be kept to avoid referencing freed memory. Unless there is another option. -- 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=21568 --- Comment #5 from Juan Lang <juan_lang(a)yahoo.com> 2010-07-12 11:56:37 --- (In reply to comment #4)
OK. So wine needs to be fixed. Is there a way to know if a pointer has been freed? Just looking through MSDN it seems there isn't. If that's the case then a list of valid handles will need to be kept to avoid referencing freed memory. Unless there is another option.
Wine doesn't need to be fixed, the tests need to be fixed not to crash when +heap is enabled. The app itself is buggy, there's no guarantee it wouldn't crash with a different heap implementation under Windows. -- 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=21568 --- Comment #6 from Austin Lund <austin.lund(a)gmail.com> 2010-07-12 21:44:27 --- (In reply to comment #5)
(In reply to comment #4)
OK. So wine needs to be fixed. Is there a way to know if a pointer has been freed? Just looking through MSDN it seems there isn't. If that's the case then a list of valid handles will need to be kept to avoid referencing freed memory. Unless there is another option.
Wine doesn't need to be fixed, the tests need to be fixed not to crash when +heap is enabled. The app itself is buggy, there's no guarantee it wouldn't crash with a different heap implementation under Windows.
That doesn't make sense to me. Under the current test windows doesn't crash and an app depends on that. Wine has the possibility of crashing and does with +heap. So either the test is removed, or wine is changed to not crash like windows. I don't understand how the test can be fixed without being removed whilst still testing the required behaviour. -- 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=21568 --- Comment #7 from Andrew Nguyen <arethusa26(a)gmail.com> 2010-07-13 03:33:13 --- It seems reasonable to believe that an HCRYPTKEY handle that CryptGenKey outputs is not a direct pointer to a data structure. Given that it's typedefed as a ULONG_PTR, and doing something (admittedly crude) like: HCRYPTKEY p; BOOL result; for (p = 0; p < 0x00400000; p++) { result = pCryptDestroyKey(p); ok (!result && GetLastError() == ERROR_INVALID_PARAMETER, "%d\n", GetLastError()); } does not induce a crash, nor is there any evidence of the process heap being corrupted (HeapValidate), this seems to suggest that some sort of handle table should be employed. -- 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=21568 --- Comment #8 from Austin Lund <austin.lund(a)gmail.com> 2010-08-05 03:46:58 --- Created an attachment (id=30013) --> (http://bugs.winehq.org/attachment.cgi?id=30013) Adding handle table for HCRYPTKEY I've tried to add a handle table implementation. I've only done it for HCRYPTKEY. It does make that particular function call not crash in the test, but then it crashes on a released HCRYPTHASH. So if this is the "right" approach, then it needs to be duplicated for the other handles. -- 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=21568 --- Comment #9 from Austin Lund <austin.lund(a)gmail.com> 2010-08-05 05:01:30 --- Created an attachment (id=30014) --> (http://bugs.winehq.org/attachment.cgi?id=30014) An alternative fix using SEH It was suggested that the handle table solution is ugly and a better way would be with SEH. Another positive is that all of the crashes are solved with this patch. -- 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=21568 --- Comment #10 from Austin Lund <austin.lund(a)gmail.com> 2010-08-17 20:43:21 --- The suggestion on IRC was to use a magic code. The patch for this is here: http://www.winehq.org/pipermail/wine-patches/2010-August/092123.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=21568 --- Comment #11 from Austin Lund <austin.lund(a)gmail.com> 2010-08-18 17:40:31 --- The magic values patch was committed: c98e6c09ae0073a1d73f638d6ee631984c0f528f -- 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=21568 Austin English <austinenglish(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |RESOLVED Resolution| |FIXED --- Comment #12 from Austin English <austinenglish(a)gmail.com> 2010-08-18 17:50:07 --- (In reply to comment #11)
The magic values patch was committed: c98e6c09ae0073a1d73f638d6ee631984c0f528f
And today, the test passed: http://test.winehq.org/data/83335590369da812440ecbda1c88ead24f1ec7dd/wine_ae... thanks! -- 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=21568 Alexandre Julliard <julliard(a)winehq.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |CLOSED --- Comment #13 from Alexandre Julliard <julliard(a)winehq.org> 2010-08-20 12:39:14 --- Closing bugs fixed in 1.3.1. -- 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