[PATCH v2 0/2] MR1949: Draft: win32u: Correct GetRawInputBuffer alignment under WoW64.
The existing mitigation for this (memcpy/overhead) leaves the first 8 bytes of RAWINPUT.data in the location of the wParam, and the upper 4 bytes of the wParam in the lower 4 bytes of the hDevice. Marking this as a draft due to code freeze. -- v2: win32u: Correct GetRawInputBuffer alignment under WoW64. https://gitlab.winehq.org/wine/wine/-/merge_requests/1949
From: Tim Clem <tclem(a)codeweavers.com> Exposes an alignment issue under WoW64. --- dlls/user32/tests/input.c | 43 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 42 insertions(+), 1 deletion(-) diff --git a/dlls/user32/tests/input.c b/dlls/user32/tests/input.c index bf913b8e6a6..c2b2e4ded80 100644 --- a/dlls/user32/tests/input.c +++ b/dlls/user32/tests/input.c @@ -2296,13 +2296,15 @@ static LRESULT CALLBACK rawinputbuffer_wndproc(HWND hwnd, UINT msg, WPARAM wpara static void test_GetRawInputBuffer(void) { - unsigned int size, count, rawinput_size, header_size; + unsigned int size, count, rawinput_size, header_size, scan_code; RAWINPUTDEVICE raw_devices[1]; char buffer[16 * sizeof(RAWINPUT64)]; HWND hwnd; BOOL ret; POINT pt; +#define HEADER_FIELD(field) (is_wow64 ? ((RAWINPUT64 *)buffer)->header.field : ((RAWINPUT *)buffer)->header.field) + if (is_wow64) rawinput_size = sizeof(RAWINPUT64); else rawinput_size = sizeof(RAWINPUT); @@ -2392,6 +2394,8 @@ static void test_GetRawInputBuffer(void) count = GetRawInputBuffer((RAWINPUT*)buffer, &size, sizeof(RAWINPUTHEADER)); ok(count == 1U, "GetRawInputBuffer returned %u\n", count); ok(size == sizeof(buffer), "GetRawInputBuffer returned unexpected size: %u\n", size); + ok(HEADER_FIELD(dwType) == RIM_TYPEMOUSE, "Unexpected rawinput dwType: %ld\n", HEADER_FIELD(dwType)); + ok(HEADER_FIELD(wParam) == 0 || HEADER_FIELD(wParam) == 1, "Expected wparam 0 or 1, got %Iu\n", (WPARAM)HEADER_FIELD(wParam)); ok(rawinput_buffer_mouse_x(buffer, 0) == 5, "Unexpected rawinput data: %d\n", rawinput_buffer_mouse_x(buffer, 0)); @@ -2428,7 +2432,44 @@ static void test_GetRawInputBuffer(void) ok(ret, "RegisterRawInputDevices failed\n"); ok(GetLastError() == 0xdeadbeef, "RegisterRawInputDevices returned %08lx\n", GetLastError()); + + /* some keyboard tests to better check fields under wow64 */ + raw_devices[0].usUsagePage = 0x01; + raw_devices[0].usUsage = 0x06; + raw_devices[0].dwFlags = RIDEV_INPUTSINK; + raw_devices[0].hwndTarget = hwnd; + + SetLastError(0xdeadbeef); + ret = RegisterRawInputDevices(raw_devices, ARRAY_SIZE(raw_devices), sizeof(RAWINPUTDEVICE)); + ok(ret, "RegisterRawInputDevices failed\n"); + ok(GetLastError() == 0xdeadbeef, "RegisterRawInputDevices returned %08lx\n", GetLastError()); + + keybd_event('X', 0x2d, 0, 0); + keybd_event('X', 0x2d, KEYEVENTF_KEYUP, 0); + + size = sizeof(buffer); + memset(buffer, 0, sizeof(buffer)); + count = GetRawInputBuffer((RAWINPUT*)buffer, &size, sizeof(RAWINPUTHEADER)); + ok(count == 2U, "GetRawInputBuffer returned %u\n", count); + ok(size == sizeof(buffer), "GetRawInputBuffer returned unexpected size: %u\n", size); + + ok(HEADER_FIELD(dwType) == RIM_TYPEKEYBOARD, "Unexpected rawinput dwType: %ld\n", HEADER_FIELD(dwType)); + todo_wine_if(is_wow64) + ok(HEADER_FIELD(wParam) == 0 || HEADER_FIELD(wParam) == 1, "Expected wparam 0 or 1, got %Iu\n", (WPARAM)HEADER_FIELD(wParam)); + scan_code = is_wow64 ? ((RAWINPUT64 *)buffer)->data.keyboard.MakeCode : ((RAWINPUT *)buffer)->data.keyboard.MakeCode; + ok(scan_code == 0x2d, "Unexpected rawinput keyboard scan code: %x\n", scan_code); + + raw_devices[0].dwFlags = RIDEV_REMOVE; + raw_devices[0].hwndTarget = 0; + + SetLastError(0xdeadbeef); + ret = RegisterRawInputDevices(raw_devices, ARRAY_SIZE(raw_devices), sizeof(RAWINPUTDEVICE)); + ok(ret, "RegisterRawInputDevices failed\n"); + ok(GetLastError() == 0xdeadbeef, "RegisterRawInputDevices returned %08lx\n", GetLastError()); + DestroyWindow(hwnd); + +#undef HEADER_FIELD } static BOOL rawinput_test_received_legacy; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1949
From: Tim Clem <tclem(a)codeweavers.com> Patch originally by Rémi Bernon. --- dlls/user32/tests/input.c | 1 - dlls/win32u/rawinput.c | 12 +++++++++++- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/dlls/user32/tests/input.c b/dlls/user32/tests/input.c index c2b2e4ded80..95abdbb601b 100644 --- a/dlls/user32/tests/input.c +++ b/dlls/user32/tests/input.c @@ -2454,7 +2454,6 @@ static void test_GetRawInputBuffer(void) ok(size == sizeof(buffer), "GetRawInputBuffer returned unexpected size: %u\n", size); ok(HEADER_FIELD(dwType) == RIM_TYPEKEYBOARD, "Unexpected rawinput dwType: %ld\n", HEADER_FIELD(dwType)); - todo_wine_if(is_wow64) ok(HEADER_FIELD(wParam) == 0 || HEADER_FIELD(wParam) == 1, "Expected wparam 0 or 1, got %Iu\n", (WPARAM)HEADER_FIELD(wParam)); scan_code = is_wow64 ? ((RAWINPUT64 *)buffer)->data.keyboard.MakeCode : ((RAWINPUT *)buffer)->data.keyboard.MakeCode; ok(scan_code == 0x2d, "Unexpected rawinput keyboard scan code: %x\n", scan_code); diff --git a/dlls/win32u/rawinput.c b/dlls/win32u/rawinput.c index ab1252ccea6..1ddbca0896a 100644 --- a/dlls/win32u/rawinput.c +++ b/dlls/win32u/rawinput.c @@ -671,10 +671,20 @@ UINT WINAPI NtUserGetRawInputBuffer( RAWINPUT *data, UINT *data_size, UINT heade if (!rawinput_from_hardware_message( data, msg_data )) break; if (overhead) { + /* Under WoW64, GetRawInputBuffer always gives 64-bit RAWINPUT structs. */ + RAWINPUT64 *ri64 = (RAWINPUT64 *)data; memmove( (char *)&data->data + overhead, &data->data, data->header.dwSize - sizeof(RAWINPUTHEADER) ); + ri64->header.dwSize += overhead; + + /* Need to copy wParam before hDevice so it's not overwritten. */ + ri64->header.wParam = data->header.wParam; +#ifdef _WIN64 + ri64->header.hDevice = data->header.hDevice; +#else + ri64->header.hDevice = HandleToULong(data->header.hDevice); +#endif } - data->header.dwSize += overhead; remaining -= data->header.dwSize; data = NEXTRAWINPUTBLOCK(data); msg_data = (struct hardware_msg_data *)((char *)msg_data + msg_data->size); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1949
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=128400 Your paranoid android. === w7u_el (32 bit report) === user32: input.c:4583: Test failed: SendInput triggered unexpected message 0xc042
On Fri Jan 13 07:11:10 2023 +0000, Rémi Bernon wrote:
(Well, I wanted to make it simpler, but I realize that __align high bits are left untouched here, probably better to keep the #ifdef and HandleToUlong) Oh, that's significantly better. Thanks for the suggestion. v2 is up.
One gotcha is that we need to copy the wParam over to the 64-bit struct before the hDevice, or it gets overwritten. Not that we set it yet anyway. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1949#note_20930
This merge request was approved by Rémi Bernon. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1949
Fwiw, I think it should be safe to merge this into 8.0, so probably you can remove the draft. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1949#note_20936
participants (4)
-
Marvin -
Rémi Bernon -
Tim Clem
-
Tim Clem (＠tclem)