[Bug 54661] New: winscard:winscard - The 32-bit test crashes in Wine due to incorrect libpcsclite function signatures

[WineHQ Bugzilla]

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: [e-mail]
          Reporter: [e-mail]
      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 
AuthorDate: Thu Feb 16 09:56:12 2023 +0100

winscard: Implement SCardEstablish/ReleaseContext() on top of libpcsclite.

[Bug 54661] winscard:winscard - The 32-bit test crashes in Wine due to incorrect libpcsclite function signatures

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=54661

François Gouget  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|[e-mail]        |[e-mail]
           Keywords|                            |regression, source,
                   |                            |testcase
    Regression SHA1|                            |8490c43f38e306fef7b5fed3ffc
                   |                            |b256efd73af58

[Bug 54661] winscard:winscard - The 32-bit test crashes in Wine due to incorrect libpcsclite function signatures

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=54661

--- Comment #1 from François Gouget  ---
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.

[Bug 54661] winscard:winscard - The 32-bit test crashes in Wine due to incorrect libpcsclite function signatures

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=54661

--- Comment #2 from Hans Leidekker  ---
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.

[Bug 54661] winscard:winscard - The 32-bit test crashes in Wine due to incorrect libpcsclite function signatures

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=54661

--- Comment #3 from François Gouget  ---
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.

[Bug 54661] winscard:winscard - The 32-bit test crashes in Wine due to incorrect libpcsclite function signatures

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=54661

--- Comment #4 from Hans Leidekker  ---
(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.

[Bug 54661] winscard:winscard - The 32-bit test crashes in Wine due to incorrect libpcsclite function signatures

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=54661

--- Comment #5 from Hans Leidekker  ---
A related issue that the 32-bit development package has dependency problems, on
64-bit Debian at least.

[Bug 54661] winscard:winscard - The 32-bit test crashes in Wine due to incorrect libpcsclite function signatures

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=54661

François Gouget  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
    Regression SHA1|8490c43f38e306fef7b5fed3ffc |77fdfdb8ef09153bf0adbfa6226
                   |b256efd73af58               |b6477b4dd0702

--- Comment #6 from François Gouget  ---
(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

[Bug 54661] winscard:winscard - The 32-bit test crashes in Wine due to incorrect libpcsclite function signatures

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=54661

--- Comment #7 from Hans Leidekker  ---
(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.

[Bug 54661] winscard:winscard - The 32-bit test crashes in Wine due to incorrect libpcsclite function signatures

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=54661

--- Comment #8 from François Gouget  ---
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.

[Bug 54661] winscard:winscard - The 32-bit test crashes in Wine due to incorrect libpcsclite function signatures

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=54661

François Gouget  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|[e-mail]     |[e-mail]

[Bug 54661] winscard:winscard - The 32-bit test crashes in Wine due to incorrect libpcsclite function signatures

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=54661

yan12125  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #9 from yan12125  ---
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

[Bug 54661] winscard:winscard - The 32-bit test crashes in Wine due to incorrect libpcsclite function signatures

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=54661

--- Comment #10 from François Gouget  ---
(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.

[Bug 54661] winscard:winscard - The 32-bit test crashes in Wine due to incorrect libpcsclite function signatures

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=54661

--- Comment #11 from Hans Leidekker  ---
(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.

[Bug 54661] winscard:winscard - The 32-bit test crashes in Wine due to incorrect libpcsclite function signatures

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=54661

--- Comment #12 from François Gouget  ---
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.

[Bug 54661] winscard:winscard - The 32-bit test crashes in Wine due to incorrect libpcsclite function signatures

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=54661

--- Comment #13 from yan12125  ---
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...

[Bug 54661] winscard:winscard - The 32-bit test crashes in Wine due to incorrect libpcsclite function signatures

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=54661

Hans Leidekker  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |WONTFIX

--- Comment #14 from Hans Leidekker  ---
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.

[Bug 54661] winscard:winscard - The 32-bit test crashes in Wine due to incorrect libpcsclite function signatures

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=54661

Zeb Figura  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #15 from Zeb Figura  ---
(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.

[Bug 54661] winscard:winscard - The 32-bit test crashes in Wine due to incorrect libpcsclite function signatures

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=54661

--- Comment #16 from Zeb Figura  ---
(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.

[Bug 54661] winscard:winscard - The 32-bit test crashes in Wine due to incorrect libpcsclite function signatures

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=54661

Austin English  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |CLOSED

--- Comment #17 from Austin English  ---
Closing.

[Bug 54661] winscard:winscard - The 32-bit test crashes in Wine due to incorrect libpcsclite function signatures

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=54661

yan12125  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|[e-mail]   |