[PATCH v2 0/2] MR1949: Draft: win32u: Correct GetRawInputBuffer alignment under WoW64.

[Tim Clem (＠tclem)]

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

[PATCH v2 1/2] user32/tests: Test GetRawInputBuffer header fields more thoroughly.

[Tim Clem]

From: Tim Clem

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;

[PATCH v2 2/2] win32u: Correct GetRawInputBuffer alignment under WoW64.

[Tim Clem]

From: Tim Clem

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);

[PATCH v2 2/2] win32u: Correct GetRawInputBuffer alignment under WoW64.

[Marvin]

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

[Tim Clem (＠tclem)]

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.

[PATCH v2 0/2] MR1949: Draft: win32u: Correct GetRawInputBuffer alignment under WoW64. - approved

[Rémi Bernon (＠rbernon)]

This merge request was approved by Rémi Bernon.

[Rémi Bernon (＠rbernon)]

Fwiw, I think it should be safe to merge this into 8.0, so probably you can remove the draft.