[PATCH 0/1] MR9187: Fix native TWAIN transfer mode for 64-Bit wine: pData points to a HGLOBAL,...
Fix native TWAIN transfer mode for 64-Bit wine: pData points to a HGLOBAL, which is a 64-Bit value in WIN64, not an UINT32 -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9187
From: Bernd Herd <codeberg(a)herdsoft.com> --- dlls/sane.ds/ds_image.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dlls/sane.ds/ds_image.c b/dlls/sane.ds/ds_image.c index 250b4aba6f9..316fcceffeb 100644 --- a/dlls/sane.ds/ds_image.c +++ b/dlls/sane.ds/ds_image.c @@ -296,7 +296,7 @@ TW_UINT16 SANE_ImageNativeXferGet (pTW_IDENTITY pOrigin, TW_MEMREF pData) { TW_UINT16 twRC = TWRC_SUCCESS; - pTW_UINT32 pHandle = (pTW_UINT32) pData; + TW_HANDLE *pHandle = (TW_HANDLE *) pData; HANDLE hDIB; BITMAPINFOHEADER *header = NULL; int dib_bytes; @@ -458,7 +458,7 @@ TW_UINT16 SANE_ImageNativeXferGet (pTW_IDENTITY pOrigin, } SANE_CALL( cancel_device, NULL ); - *pHandle = (UINT_PTR)hDIB; + *pHandle = (TW_HANDLE)hDIB; twRC = TWRC_XFERDONE; activeDS.twCC = TWCC_SUCCESS; activeDS.currentState = 7; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9187
AFAICT it points to an HBITMAP, which is also a pointer-sized type but even in a 64-bit process will always fit in 32 bits. The headers don't make it very clear what's expected. I did a code search on GitHub and didn't see any consistency around how it's handled. I'd suggest testing this on Windows, but I don't have a way to do that, and it might depend on the driver. If an application uses a 64-bit variable that it initializes to NULL, that should work either way. If an application uses a 64-bit variable that it doesn't initialize, and we write 32 bits, the high DWORD will be undefined. Based on my reading of gdi32's `handle_entry` function, I think it'll still accept the value, so long as the application itself doesn't check the high DWORD for whatever reason. If a 64-bit application uses a 32-bit variable, and we write 64 bits, we'll corrupt memory. So if we're not sure, the safe option would be to leave it as-is. Do you have any more information that indicates we should write this as a 64-bit value? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9187#note_118808
First all all thank you for your time spent to analyze the situation. However I do not agree with your results and here you'll find detailed prove that my suggested patch makes sense: The reason I devloped this patch is that my 25 years old TWAIN code crashed with this driver when compiled for 64-Bit WIN32, since the returned handle was missing the upper 32 bits. --- ## 1st line of evidence: The handle is a HGLOBAL-Handle in DIB-Format. So it is a memory block allocated with GlobalAlloc. You can see this from the source that I've patched: dlls/sane.ds/ds_image.c:301 (In the version as patched by me): `TW_HANDLE *pHandle = (TW_HANDLE *) pData;` `HANDLE hDIB; ` `[...] ` `hDIB = GlobalAlloc(GMEM_ZEROINIT, dib_bytes + sizeof(*header) + color_size);` `[...]` `*pHandle = (TW_HANDLE)hDIB;` --- ## `2nd line of evidence:` You may find the current TWAIN specification here: https://github.com/twain/twain-specification/blob/master/versions/2.4/TWAIN-... On page 53 the DAT_IMAGENATIVEXFER is described: "pData Points to an OS specific native Image format returned by the Data Source.\[...\] On Windows: The Source will set pData to point to a device-independent bitmap (DIB) that it allocates." As a description, what a device-independent bitmap handle is, you might reference to: https://learn.microsoft.com/en-us/windows/win32/dataxchg/standard-clipboard-... And the description of CF_DIB: "A memory object containing a BITMAPINFO structure followed by the bitmap bits." --- ### 3rd line of evidence: My code is here: https://codeberg.org/herdsoft/davinci4/src/branch/master/leonardo/twainac.c Around line 402. At line 421 there is a GlobalLock performed on the returned handle, and here it crashes without the patch to ds_image.c. --- ## 4th line of evidence: The official TWAIN example program from the TWAIN working group: TW_MEMREF in twain.h is a "typedef LPVOID TW_MEMREF"; https://github.com/twain/twain-samples/blob/master/TWAIN-Samples/Twain_App_s... At line 880: `TW_MEMREF hImg = 0;` `[...]` `twrc = DSM_Entry( DG_IMAGE, DAT_IMAGENATIVEXFER, MSG_GET, (TW_MEMREF)&hImg);` `[...] ` `PBITMAPINFOHEADER pDIB = (PBITMAPINFOHEADER)_DSM_LockMemory(hImg);` The spec says on Page 643, that when using a "legacy TWAIN 1.x DSM" the app shall use GlobalAlloc, GlobalFree, GlobalLock, GlobalUnlock and when using the 2.x DSM it must use the "memory functions supplied by the DSM." So here the source of the official TWAIN dsm: https://github.com/twain/twain-dsm/blob/master/TWAIN_DSM/src/twain.h Line 2265 describes how the function pointers are to be transfered. https://github.com/twain/twain-dsm/blob/master/TWAIN_DSM/src/dsm.cpp Line 3829 the current implementation inside of the DSM: `TW_MEMREF PASCAL DSM_MemLock (TW_HANDLE _handle) ` `{` ` [...]` ` return (TW_MEMREF)::GlobalLock(_handle);` --- So it is crystal clear, that the existing code in the dlls/sane.ds/ds_image.c SANE_ImageNativeXferGet is using the wrong data type for 64 Bit. However you'll find few 64-Bit TWAIN drivers and few 64-Bit windows applications that support 64-Bit twain drivers since Microsoft does now offer the WIA (Windows Image Aquisition) interface. For 32-Bit applications, Windows has a WIA\<-\>TWAIN compatibility layer, but 64-Bit windows applications need to implement WIA. The TWAIN group offers a 64-Bit version of the DSM, but as far as I can tell, hardly any vendor is providing 64-Bit TWAIN drivers. So there will be few Windows Applications that support 64-Bit Wine. But with that patch applied to ds_image.c, my old TWAIN code compiled for 64-Bit WIN32 is able to scan from my scanner and sane, even if it does not work with Microsoft Windows. See for example: https://www.irfanview.com/64bit.htm So it seems likely, that 64-Bit Irfanview and others might also profit from my patch. ## sizeof(HBITMAP)!=sizeof(INT32) I'd also like to point out that sizeof(HBITMAP) is also 8 in WIN64, so even if you'd be right that it would be a HBITMAP handle and not a HGLOBAL handle, it would still be wrong: `echo '#include <windows.h>` `#include <stdio.h>` `int main(int argc, char **atv)` `{` ` printf("sizeof(HITMAP)=%d ", sizeof(HBITMAP)); ` ` printf("sizeof(HGLOBAL)=%d\n", sizeof(HGLOBAL));` ` return 0; ` `}' >handle_size.c` `x86_64-w64-mingw32-gcc -o handle_size.exe handle_size.c && wine handle_size.exe sizeof(HITMAP)=8 sizeof(HGLOBAL)=8` `i686-w64-mingw32-gcc -o handle_size.exe handle_size.c && wine handle_size.exe sizeof(HITMAP)=4 sizeof(HGLOBAL)=4` --- I've got one more patch to the TWAIN interface on WINE in mind, since I happen to know a few things about TWAIN and some details are implemented in a wrong way, but they are of little importance. When I encountered the crash with the native transfer mode, I tried the buffered memory transfer mode. And that failed for other reasons that I could develop a patch for. And that also applies to the 32-Bit version. All the Best. Bernd Herd :-) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9187#note_118904
On Fri Oct 17 22:08:58 2025 +0000, Bernd Herd wrote:
First all all thank you for your time spent to analyze the situation. However I do not agree with your results and here you'll find detailed prove that my suggested patch makes sense: The reason I devloped this patch is that my 25 years old TWAIN code crashed with this driver when compiled for 64-Bit WIN32, since the returned handle was missing the upper 32 bits. --- ## 1st line of evidence: The handle is a HGLOBAL-Handle in DIB-Format. So it is a memory block allocated with GlobalAlloc. You can see this from the source that I've patched: dlls/sane.ds/ds_image.c:301 (In the version as patched by me): `TW_HANDLE *pHandle = (TW_HANDLE *) pData;` `HANDLE hDIB; ` `[...] ` `hDIB = GlobalAlloc(GMEM_ZEROINIT, dib_bytes + sizeof(*header) + color_size);` `[...]` `*pHandle = (TW_HANDLE)hDIB;` --- ## `2nd line of evidence:` You may find the current TWAIN specification here: https://github.com/twain/twain-specification/blob/master/versions/2.4/TWAIN-... On page 53 the DAT_IMAGENATIVEXFER is described: "pData Points to an OS specific native Image format returned by the Data Source.\[...\] On Windows: The Source will set pData to point to a device-independent bitmap (DIB) that it allocates." As a description, what a device-independent bitmap handle is, you might reference to: https://learn.microsoft.com/en-us/windows/win32/dataxchg/standard-clipboard-... And the description of CF_DIB: "A memory object containing a BITMAPINFO structure followed by the bitmap bits." --- ### 3rd line of evidence: My code is here: https://codeberg.org/herdsoft/davinci4/src/branch/master/leonardo/twainac.c Around line 402. At line 421 there is a GlobalLock performed on the returned handle, and here it crashes without the patch to ds_image.c. --- ## 4th line of evidence: The official TWAIN example program from the TWAIN working group: TW_MEMREF in twain.h is a "typedef LPVOID TW_MEMREF"; https://github.com/twain/twain-samples/blob/master/TWAIN-Samples/Twain_App_s... At line 880: `TW_MEMREF hImg = 0;` `[...]` `twrc = DSM_Entry( DG_IMAGE, DAT_IMAGENATIVEXFER, MSG_GET, (TW_MEMREF)&hImg);` `[...] ` `PBITMAPINFOHEADER pDIB = (PBITMAPINFOHEADER)_DSM_LockMemory(hImg);` The spec says on Page 643, that when using a "legacy TWAIN 1.x DSM" the app shall use GlobalAlloc, GlobalFree, GlobalLock, GlobalUnlock and when using the 2.x DSM it must use the "memory functions supplied by the DSM." So here the source of the official TWAIN dsm: https://github.com/twain/twain-dsm/blob/master/TWAIN_DSM/src/twain.h Line 2265 describes how the function pointers are to be transfered. https://github.com/twain/twain-dsm/blob/master/TWAIN_DSM/src/dsm.cpp Line 3829 the current implementation inside of the DSM: `TW_MEMREF PASCAL DSM_MemLock (TW_HANDLE _handle) ` `{` ` [...]` ` return (TW_MEMREF)::GlobalLock(_handle);` --- So it is crystal clear, that the existing code in the dlls/sane.ds/ds_image.c SANE_ImageNativeXferGet is using the wrong data type for 64 Bit. However you'll find few 64-Bit TWAIN drivers and few 64-Bit windows applications that support 64-Bit twain drivers since Microsoft does now offer the WIA (Windows Image Aquisition) interface. For 32-Bit applications, Windows has a WIA\<-\>TWAIN compatibility layer, but 64-Bit windows applications need to implement WIA. The TWAIN group offers a 64-Bit version of the DSM, but as far as I can tell, hardly any vendor is providing 64-Bit TWAIN drivers. So there will be few Windows Applications that support 64-Bit Wine. But with that patch applied to ds_image.c, my old TWAIN code compiled for 64-Bit WIN32 is able to scan from my scanner and sane, even if it does not work with Microsoft Windows. See for example: https://www.irfanview.com/64bit.htm So it seems likely, that 64-Bit Irfanview and others might also profit from my patch. ## sizeof(HBITMAP)!=sizeof(INT32) I'd also like to point out that sizeof(HBITMAP) is also 8 in WIN64, so even if you'd be right that it would be a HBITMAP handle and not a HGLOBAL handle, it would still be wrong: `echo '#include <windows.h>` `#include <stdio.h>` `int main(int argc, char **atv)` `{` ` printf("sizeof(HITMAP)=%d ", sizeof(HBITMAP)); ` ` printf("sizeof(HGLOBAL)=%d\n", sizeof(HGLOBAL));` ` return 0; ` `}' >handle_size.c` `x86_64-w64-mingw32-gcc -o handle_size.exe handle_size.c && wine handle_size.exe sizeof(HITMAP)=8 sizeof(HGLOBAL)=8` `i686-w64-mingw32-gcc -o handle_size.exe handle_size.c && wine handle_size.exe sizeof(HITMAP)=4 sizeof(HGLOBAL)=4` --- I've got one more patch to the TWAIN interface on WINE in mind, since I happen to know a few things about TWAIN and some details are implemented in a wrong way, but they are of little importance. When I encountered the crash with the native transfer mode, I tried the buffered memory transfer mode. And that failed for other reasons that I could develop a patch for. And that also applies to the 32-Bit version. All the Best. Bernd Herd :-) Ah, sorry. I completely missed that this returns an HGLOBAL, not an HBITMAP, and thus there's no reason to expect it'll fit in the lower 32 bits. So applications clearly need a 64-bit variable, and we can't implement it correctly without assigning to one.
I think that confusing comment in the header about it being a TW_UINT32 (which exists in version 1.7 of twain.h but not the current version) must have been from before 64-bit architectures were common. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9187#note_118971
This merge request was approved by Esme Povirk. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9187
Oddly, gphoto2.ds does return an HBITMAP for this operation, and it has a comment implying they're supposed to be equivalent to HGLOBAL: https://gitlab.winehq.org/wine/wine/-/blob/master/dlls/gphoto2.ds/ds_image.c... It also only returns 32 bits. I have never heard of GlobalLock working with HBITMAPs, though. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9187#note_118973
On Fri Oct 17 22:22:54 2025 +0000, Esme Povirk wrote:
Oddly, gphoto2.ds does return an HBITMAP for this operation, and it has a comment implying they're supposed to be equivalent to HGLOBAL: https://gitlab.winehq.org/wine/wine/-/blob/master/dlls/gphoto2.ds/ds_image.c... It also only returns 32 bits. I have never heard of GlobalLock working with HBITMAPs, though. I think we should update gphoto2.ds to return an HGLOBAL in a similar way to this.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9187#note_118974
participants (3)
-
Bernd Herd -
Bernd Herd (＠herdsoft)
-
Esme Povirk (＠madewokherd)