[PATCH 0/1] MR1364: win32u: Correctly report count in GetRawInputBuffer

[Aric Stewart (＠aricstewart)]

If rawinput_from_hardware_message generates an error then the number of
packets handled may be less then the size of the buffer. Correctly
report only the successfully processed packets to avoid using
uninitialized data.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/1364

[PATCH 1/1] win32u: Correctly report count in GetRawInputBuffer

[Aric Stewart]

From: Aric Stewart <aric(a)codeweavers.com>

If rawinput_from_hardware_message generates an error then the number of
packets handled may be less then the size of the buffer. Correctly
report only the successfully processed packets to avoid using
uninitialized data.
---
 dlls/win32u/rawinput.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/dlls/win32u/rawinput.c b/dlls/win32u/rawinput.c
index 8795ebbba00..585eea9f3e1 100644
--- a/dlls/win32u/rawinput.c
+++ b/dlls/win32u/rawinput.c
@@ -609,7 +609,7 @@ UINT WINAPI NtUserGetRawInputDeviceInfo( HANDLE handle, UINT command, void *data
  */
 UINT WINAPI NtUserGetRawInputBuffer( RAWINPUT *data, UINT *data_size, UINT header_size )
 {
-    unsigned int count = 0, remaining, rawinput_size, next_size, overhead;
+    unsigned int count = 0, remaining, rawinput_size, next_size, overhead, handled;
     struct rawinput_thread_data *thread_data;
     struct hardware_msg_data *msg_data;
     RAWINPUT *rawinput;
@@ -665,6 +665,7 @@ UINT WINAPI NtUserGetRawInputBuffer( RAWINPUT *data, UINT *data_size, UINT heade
     SERVER_END_REQ;
 
     remaining = *data_size;
+    handled = 0;
     for (i = 0; i < count; ++i)
     {
         data->header.dwSize = remaining;
@@ -678,11 +679,12 @@ UINT WINAPI NtUserGetRawInputBuffer( RAWINPUT *data, UINT *data_size, UINT heade
         remaining -= data->header.dwSize;
         data = NEXTRAWINPUTBLOCK(data);
         msg_data = (struct hardware_msg_data *)((char *)msg_data + msg_data->size);
+        handled++;
     }
 
     if (!next_size)
     {
-        if (!count)
+        if (!handled)
             *data_size = 0;
         else
             next_size = rawinput_size;
@@ -692,12 +694,12 @@ UINT WINAPI NtUserGetRawInputBuffer( RAWINPUT *data, UINT *data_size, UINT heade
     {
         RtlSetLastWin32Error( ERROR_INSUFFICIENT_BUFFER );
         *data_size = next_size;
-        count = ~0u;
+        handled = count = ~0u;
     }
 
-    TRACE( "data %p, data_size %p (%u), header_size %u, count %u\n",
-           data, data_size, *data_size, header_size, count );
-    return count;
+    TRACE( "data %p, data_size %p (%u), header_size %u, count %u(%u)\n",
+           data, data_size, *data_size, header_size, count, handled );
+    return handled;
 }
 
 /**********************************************************************
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/1364

[PATCH 1/1] win32u: Correctly report count in GetRawInputBuffer

[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=126159

Your paranoid android.


=== debian11 (32 bit report) ===

crypt32:
cert.c:4191: Test failed: success
cert.c:4192: Test failed: got 00000000
cert.c:4193: Test failed: got 00000000

[Rémi Bernon]

I'm a bit surprised that `rawinput_from_hardware_message` returns an error, especially in `GetRawInputBuffer` where we should have requested the correct number of elements from the server call, do you have more detail why it happens?

The issue I'm seeing is that if we requested too many elements from the server, I think that breaking the loop short will drop them.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/1364#note_15986

[Aric Stewart (＠aricstewart)]

What I am seeing in this app is a number of message like this.

`37513.309:0130:0134:fixme:rawinput:rawinput_from_hardware_message Unhandled rawinput type 0x100.`

So somehow there are invalid rawinput message coming in from the controller level. 
I have not investigated down to that level yet as the handling of such an error in GetRawInputBuffer already looks like it needs fixing. And with that fixed The controller functions as expected.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/1364#note_16008

[Rémi Bernon]

Okay, but this is then only hiding the problem, that kind of error should really never happen as it's not something sent by the controller but instead the value of `RIM_TYPE*` constant. All the possible values are normally handled already (`RIM_TYPEMOUSE`, `RIM_TYPEKEYBOARD`, `RIM_TYPEHID`).

I believe we are the ones settings these types and if there's an unexpected type it is that something is going wrong elsewhere and upstream in the HID / input stack, that needs to be fixed.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/1364#note_16012