We need to memmove 11 bytes, otherwise `buffer[10]` ends up as a copy of `buffer[9]` which resulted in the second byte of button state being wrongly re-used as a third byte as well.
The original code did not suffer from this - while the length check was also one off, and likely informed the one off memmove, the buffer pointer was just incremented so everything "moved" correctly.
Fixes: c708295ed6de ("winebus: Move Sony controllers report fixups to PE side.")<br/> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=56883<br/> Signed-off-by: Arkadiusz Hiler <ahiler@codeweavers.com></br>
From: Csányi István icsanyi96@gmail.com
We need to memmove 11 bytes, otherwise buffer[10] ends up as a copy of buffer[9] which resulted in the second byte of button state being wrongly re-used as a third byte as well.
The original code did not suffer from this - while the length check was also one off, and likely informed the one off memmove, the buffer pointer was just incremented so everything "moved" correctly.
Fixes: c708295ed6de ("winebus: Move Sony controllers report fixups to PE side.") Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=56883 Signed-off-by: Arkadiusz Hiler ahiler@codeweavers.com --- dlls/winebus.sys/main.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/dlls/winebus.sys/main.c b/dlls/winebus.sys/main.c index cb9104ba056..f7f773e7fde 100644 --- a/dlls/winebus.sys/main.c +++ b/dlls/winebus.sys/main.c @@ -586,11 +586,11 @@ static void process_hid_report(DEVICE_OBJECT *device, BYTE *report_buf, DWORD re * Extended #41 report: * Prefix X Y Z Rz TriggerLeft TriggerRight Counter Buttons[3] ... */ - if (report->buffer[0] == 0x31 && report->length >= 11) + if (report->buffer[0] == 0x31 && report->length >= 12) { BYTE trigger[2];
- memmove(report->buffer, report->buffer + 1, 10); + memmove(report->buffer, report->buffer + 1, 11); report->buffer[0] = 1; /* fake report #1 */ report->length = 10;
This merge request was approved by Arek Hiler.
What I don't understand is how this is a regression from the indicated commit.
We were previously calling `bus_event_queue_input_report(buf, size)` with `buf = report_buffer + 1` and `size = 10`, which ended up in `process_hid_report` with `report_buf = buf` and `report_len = 10`.
Now, the fixup happens in `process_hid_report`, but the code does the same thing as before: we offset report data to `report_buf + 1`, and set report length to 10. The checks have been moved from the unix side too.
This merge request was approved by Rémi Bernon.
On Thu Jul 31 09:30:48 2025 +0000, Rémi Bernon wrote:
What I don't understand is how this is a regression from the indicated commit. We were previously calling `bus_event_queue_input_report(buf, size)` with `buf = report_buffer + 1` and `size = 10`, which ended up in `process_hid_report` with `report_buf = buf` and `report_len = 10`. Now, the fixup happens in `process_hid_report`, but the code does the same thing as before: we offset report data to `report_buf + 1`, and set report length to 10. The checks have been moved from the unix side too.
Sorry, I'm dumb, we were not memmoving the data before, that's what causing the problem.
-
Arek Hiler (＠ivyl) -
Csányi István
-
Rémi Bernon (＠rbernon)