(smartcardio) Confusing bug fix for wrong DWORD typedef

[Yonathan]

In a commit about a month ago[1], Ivan Gerasimov fixed a long-standing
bug in using javax.smartcardio on OS X that I previously mentioned[2].
I am uncomfortable with this fix, because to determine whether it was
successful requires study of x86_64 calling conventions.

Recall that the bug was that LPDWORD is typedef’d to unsigned long* in
pcsc.c, but the dylib is actually expecting uint32_t*. The “fix” was
to initialize variables with 0 before each function call. Instead, the
fix should have been to typedef DWORD to uint32_t to match the library
header. Consider this library declaration:

LONG SCardStatus(SCARDHANDLE hCard,
        LPTSTR mszReaderNames, LPDWORD pcchReaderLen,
        LPDWORD pdwState,
        LPDWORD pdwProtocol,
        LPBYTE pbAtr, LPDWORD pcbAtrLen);

When pcsc.c calls the function, does the function read/write the
correct addresses? You have to think about the calling convention:

hCard: %rdi → 8B stack variable; lib reads 4B little end

mszReaderNames: %rsi → byte buffer on stack; lib read/writes bytes

pcchReaderLen: %rdx → 8B stack variable; lib read/writes 4B little end
unsigned and we interpret it as 8B long

pdwState: %rcx → 8B stack variable; lib writes 4B little end bitmask

pdwProtocol: %r8 → 8B stack variable; lib writes 4B little end enum

pbAtr: %r9 → byte buffer on stack; lib writes bytes

pcbAtrLen: 8B (%esp) → 8B stack variable; lib writes 4B little end and
we interpret it as 8B long

Because we assume x86_64 with 8-byte registers/8 byte parameter
alignment and little-endian integers and all of them are unsigned, and
the variables we write are bigger than the variables that the library
reads, the fix should work, I think.

But why not just typedef DWORD to match the typedef in the PCSC system
header[3] on OS X so we don’t have to go through this analysis?
Moreover, there is no comment in the code to state these assumptions,
so this apparently redundant initialization is confusing.

By the way, you can also simplify the related struct declaration
fix[4] by using the same DWORD in it instead of declaring it in two
different places.

Thank you for your consideration.

Yonathan

[1] http://mail.openjdk.java.net/pipermail/security-dev/2014-May/010515.html
[2] http://mail.openjdk.java.net/pipermail/security-dev/2013-March/006913.html
[3] /System/Library/Frameworks/PCSC.framework/Headers/wintypes.h
[4] http://mail.openjdk.java.net/pipermail/security-dev/2014-May/010498.html