[Bug 54661] New: winscard:winscard - The 32-bit test crashes in Wine due to incorrect libpcsclite function signatures
https://bugs.winehq.org/show_bug.cgi?id=54661 Bug ID: 54661 Summary: winscard:winscard - The 32-bit test crashes in Wine due to incorrect libpcsclite function signatures Product: Wine Version: unspecified Hardware: x86-64 OS: Linux Status: NEW Severity: normal Priority: P2 Component: winscard Assignee: wine-bugs(a)winehq.org Reporter: fgouget(a)codeweavers.com Distribution: --- winscard:winscard - The 32-bit test crashes in Wine due to incorrect libpcsclite function signatures: winscard.c:49: Test failed: got 80100004 winscard.c:52: Test failed: got 6 winscard.c:61: Test failed: got 6 [...] winscard.c:120: Test failed: got deadbeef Unhandled exception: page fault on read access to 0xdeadbeef in 32-bit code (0x6d702b0e). See https://test.winehq.org/data/patterns.html#winscard:winscard Where 0x80100004 == SCARD_E_INVALID_PARAMETER The only important failure is the first one: SCARD_E_INVALID_PARAMETER. This happens because Wine's winscard implementation passes NULL context pointer to the libpcsclite SCardEstablishContext() function. And this happens because that function takes a 32-bit scope value while unixlib.c declares that parameter are 64-bit so the remaining parameters are all shifted. That makes the commit below the source of the bug: commit 8490c43f38e306fef7b5fed3ffcb256efd73af58 Author: Hans Leidekker <hans(a)codeweavers.com> AuthorDate: Thu Feb 16 09:56:12 2023 +0100 winscard: Implement SCardEstablish/ReleaseContext() on top of libpcsclite. -- 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.
https://bugs.winehq.org/show_bug.cgi?id=54661 François Gouget <fgouget(a)codeweavers.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|wine-bugs(a)winehq.org |fgouget(a)codeweavers.com Keywords| |regression, source, | |testcase Regression SHA1| |8490c43f38e306fef7b5fed3ffc | |b256efd73af58 -- 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.
https://bugs.winehq.org/show_bug.cgi?id=54661 --- Comment #1 from François Gouget <fgouget(a)codeweavers.com> --- Sorry, it's not the context pointer that causes libpcsclite to fail. Based on the libpcsclite code it would be the scope that ends up having a bad value. -- 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.
https://bugs.winehq.org/show_bug.cgi?id=54661 --- Comment #2 from Hans Leidekker <hans(a)meelstraat.net> --- Again, I don't see how this is a regression because there were no tests before this commit. My plan was to skip old-style 32-bit support because it gets very messy with the different DWORD sizes. -- 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.
https://bugs.winehq.org/show_bug.cgi?id=54661 --- Comment #3 from François Gouget <fgouget(a)codeweavers.com> --- Why this is a regression: there was no test and thus no test failures. Now the 32-bit tests crash which is a failure. So the Wine tests regressed. So I could point at the commit that introduced the tests but the one I pointed to is the one that will cause 32-bit Windows applications to fail. Dealing with the DWORD parameters that are passed directly to libpcsclite is easy enough. The hard part is dealing with the structures: reader_state, io_request, and the void* parameters of SCardGetAttrib() and SCardSetAttrib(). Now if the plan is to not support 32-bit code, then Wine should not provide a 32-bit winscard dll. -- 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.
https://bugs.winehq.org/show_bug.cgi?id=54661 --- Comment #4 from Hans Leidekker <hans(a)meelstraat.net> --- (In reply to François Gouget from comment #3)
Why this is a regression: there was no test and thus no test failures. Now the 32-bit tests crash which is a failure. So the Wine tests regressed.
So I could point at the commit that introduced the tests but the one I pointed to is the one that will cause 32-bit Windows applications to fail.
Most likely they would already fail since the dll was a stub.
Dealing with the DWORD parameters that are passed directly to libpcsclite is easy enough. The hard part is dealing with the structures: reader_state, io_request, and the void* parameters of SCardGetAttrib() and SCardSetAttrib().
Exactly.
Now if the plan is to not support 32-bit code, then Wine should not provide a 32-bit winscard dll.
The plan is to support 32-bit code through Wow64 thunks. -- 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.
https://bugs.winehq.org/show_bug.cgi?id=54661 --- Comment #5 from Hans Leidekker <hans(a)meelstraat.net> --- A related issue that the 32-bit development package has dependency problems, on 64-bit Debian at least. -- 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.
https://bugs.winehq.org/show_bug.cgi?id=54661 François Gouget <fgouget(a)codeweavers.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Regression SHA1|8490c43f38e306fef7b5fed3ffc |77fdfdb8ef09153bf0adbfa6226 |b256efd73af58 |b6477b4dd0702 --- Comment #6 from François Gouget <fgouget(a)codeweavers.com> --- (In reply to Hans Leidekker from comment #4)
(In reply to François Gouget from comment #3)
So I could point at the commit that introduced the tests but the one I pointed to is the one that will cause 32-bit Windows applications to fail.
Most likely they would already fail since the dll was a stub.
I guess that's a fair point. Unlike our tests they probably stop on all SCardAccessStartedEvent() errors, not just on SCARD_E_NO_SERVICE. So getting that, SCARD_F_INTERNAL_ERROR (stub), or SCARD_E_INVALID_PARAMETER (current code) probably makes no difference to them. I'm updating the regression SHA1 then. [...]
Now if the plan is to not support 32-bit code, then Wine should not provide a 32-bit winscard dll.
The plan is to support 32-bit code through Wow64 thunks.
Okay. What I meant is that then a pure 32-bit Wine build should not provide a 32-bit winscard dll. Currently this very basic command produces a crash in the tests: ./configure && make && ./wine dlls/winscard/tests/i386-windows/winscard_test.exe winscard And running the 32-bit test in a 64-bit wineprefix crashes too: https://test.winehq.org/data/patterns.html#winscard:winscard -- 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.
https://bugs.winehq.org/show_bug.cgi?id=54661 --- Comment #7 from Hans Leidekker <hans(a)meelstraat.net> --- (In reply to François Gouget from comment #6)
The plan is to support 32-bit code through Wow64 thunks.
Okay. What I meant is that then a pure 32-bit Wine build should not provide a 32-bit winscard dll. Currently this very basic command produces a crash in the tests:
./configure && make && ./wine dlls/winscard/tests/i386-windows/winscard_test.exe winscard
Right, I don't think we should install 32-bit development libraries on gitlab, just the 64-bit one. -- 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.
https://bugs.winehq.org/show_bug.cgi?id=54661 --- Comment #8 from François Gouget <fgouget(a)codeweavers.com> --- Created attachment 74177 --> https://bugs.winehq.org/attachment.cgi?id=74177 WIP winscard: Fix 32-bit support. This is enough to fix the 32-bit winscard tests (both pure 32-bit and '32-bit in a 64-bit wineprefix' as of today). However this is more involved than I initially thought and it appears this is not the right direction anyway. So I'll stop there. -- 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.
https://bugs.winehq.org/show_bug.cgi?id=54661 François Gouget <fgouget(a)codeweavers.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|fgouget(a)codeweavers.com |wine-bugs(a)winehq.org -- 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.
https://bugs.winehq.org/show_bug.cgi?id=54661 yan12125 <pq2sx44teeigl7za(a)chyen.cc> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |pq2sx44teeigl7za(a)chyen.cc --- Comment #9 from yan12125 <pq2sx44teeigl7za(a)chyen.cc> --- The SCARD_E_INVALID_PARAMETER issue is also reproducible on Arch Linux. Is the attached winscard.diff good enough to be backported to wine 8.3 on Arch? See: https://bugs.archlinux.org/task/77785 -- 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.
https://bugs.winehq.org/show_bug.cgi?id=54661 --- Comment #10 from François Gouget <fgouget(a)codeweavers.com> --- (In reply to Hans Leidekker from comment #5)
A related issue that the 32-bit development package has dependency problems, on 64-bit Debian at least.
Are these dependency issues documented anywhere? I don't remember having any issue installing libpcsclite-dev:i386 and I don't see any Debian libpcsclite-dev bug. -- 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.
https://bugs.winehq.org/show_bug.cgi?id=54661 --- Comment #11 from Hans Leidekker <hans(a)meelstraat.net> --- (In reply to François Gouget from comment #10)
(In reply to Hans Leidekker from comment #5)
A related issue that the 32-bit development package has dependency problems, on 64-bit Debian at least.
Are these dependency issues documented anywhere? I don't remember having any issue installing libpcsclite-dev:i386 and I don't see any Debian libpcsclite-dev bug.
When I try to install it apt wants to remove half of my desktop environment. -- 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.
https://bugs.winehq.org/show_bug.cgi?id=54661 --- Comment #12 from François Gouget <fgouget(a)codeweavers.com> --- I can reproduce the dependency issue on both Debian 11.6 and Debian Testing with this apt command (same with apt-get): apt install libpcsclite-dev:amd64 libpcsclite-dev:i386 It essentially wants to replace python3:amd64 with python3:i386 with cascading effects on all packages that depend on python3. However I discovered that I don't have any issue if I install libpcsclite-dev:i386 using aptitude. I also don't have any issue if I run this 'very different' apt command! apt install libpcsclite-dev:i386 libpcsclite-dev:amd64 Order... matters??? I did not know that. I also found a couple more resilient options: # Assume python3 is already installed anyway apt install --no-install-recommends libpcsclite-dev:i386 libpcsclite-dev:amd64 or # Override apt's conflict resolution algorithm apt install libpcsclite-dev:i386 libpcsclite-dev:amd64 python3+ So I'd say it's more an issue of apt being silly (why prioritize the uninstalled i386 recommends over the already installed amd64 ones) more than a libpcsclite-dev issue. So I won't file a Debian bug. -- 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.
https://bugs.winehq.org/show_bug.cgi?id=54661 --- Comment #13 from yan12125 <pq2sx44teeigl7za(a)chyen.cc> --- The recommended python dependency was added for the pcsc-spy script [1]. Is the script needed for wine tests? If not, `--no-install-recommends` may be an appropriate workaround for Debian package issues. [1] https://salsa.debian.org/debian/pcsc-lite/-/commit/4d1e774de819609f2036da375... -- 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.
https://bugs.winehq.org/show_bug.cgi?id=54661 Hans Leidekker <hans(a)meelstraat.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |RESOLVED Resolution|--- |WONTFIX --- Comment #14 from Hans Leidekker <hans(a)meelstraat.net> --- Fixing this for old-style Wow64/pure 32-bit builds is not worth it in my opinion. It complicates the code while new-style Wow64 is in a state where most applications run, so I don't think it's unreasonable to ask users to switch if they want to get smartcard support. -- 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.
https://bugs.winehq.org/show_bug.cgi?id=54661 Zeb Figura <z.figura12(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |z.figura12(a)gmail.com --- Comment #15 from Zeb Figura <z.figura12(a)gmail.com> --- (In reply to Hans Leidekker from comment #14)
It complicates the code while new-style Wow64 is in a state where most applications run,
I don't want to be argumentative, but it really isn't. 3D acceleration does not work well and isn't going to work well for probably another couple of years at least. Granted, I suppose the subset of people who need 3D support and smart card support is not going to have a high overlap. -- 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.
https://bugs.winehq.org/show_bug.cgi?id=54661 --- Comment #16 from Zeb Figura <z.figura12(a)gmail.com> --- (In reply to Zeb Figura from comment #15)
(In reply to Hans Leidekker from comment #14)
It complicates the code while new-style Wow64 is in a state where most applications run,
I don't want to be argumentative, but it really isn't. 3D acceleration does not work well and isn't going to work well for probably another couple of years at least.
Granted, I suppose the subset of people who need 3D support and smart card support is not going to have a high overlap.
But it does mean that distributions probably aren't going to enable new wow64 until that's done, and then you'd be asking people to switch who have to go out of their way to compile Wine and do so. -- 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.
https://bugs.winehq.org/show_bug.cgi?id=54661 Austin English <austinenglish(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |CLOSED --- Comment #17 from Austin English <austinenglish(a)gmail.com> --- Closing. -- 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.
https://bugs.winehq.org/show_bug.cgi?id=54661 yan12125 <pq2sx44teeigl7za(a)chyen.cc> changed: What |Removed |Added ---------------------------------------------------------------------------- CC|pq2sx44teeigl7za(a)chyen.cc | -- 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)
-
WineHQ Bugzilla