[PATCH 1/3] hidclass.sys: Use the report length when copying report data.

[Rémi Bernon]

From: Ivo Ivanov <logos128(a)gmail.com>

Instead of the HID descriptor input report length.

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=51828
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com>
---
 dlls/hidclass.sys/device.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/dlls/hidclass.sys/device.c b/dlls/hidclass.sys/device.c
index 026ec1efeb1..632e98ab740 100644
--- a/dlls/hidclass.sys/device.c
+++ b/dlls/hidclass.sys/device.c
@@ -216,7 +216,6 @@ static struct hid_report *hid_queue_pop_report( struct hid_queue *queue )
 static void hid_device_queue_input( DEVICE_OBJECT *device, HID_XFER_PACKET *packet )
 {
     BASE_DEVICE_EXTENSION *ext = device->DeviceExtension;
-    HIDP_COLLECTION_DESC *desc = ext->u.pdo.device_desc.CollectionDesc;
     const BOOL polled = ext->u.pdo.information.Polled;
     struct hid_report *last_report, *report;
     struct hid_queue *queue;
@@ -270,7 +269,7 @@ static void hid_device_queue_input( DEVICE_OBJECT *device, HID_XFER_PACKET *pack
             if (!(irp = hid_queue_pop_irp( queue ))) break;
             if (!(report = hid_queue_pop_report( queue ))) hid_report_incref( (report = last_report) );
 
-            memcpy( irp->AssociatedIrp.SystemBuffer, report->buffer, desc->InputLength );
+            memcpy( irp->AssociatedIrp.SystemBuffer, report->buffer, report->length );
             irp->IoStatus.Information = report->length;
             irp->IoStatus.Status = STATUS_SUCCESS;
             hid_report_decref( report );
@@ -652,7 +651,7 @@ NTSTATUS WINAPI pdo_read(DEVICE_OBJECT *device, IRP *irp)
     irp->IoStatus.Information = 0;
     if ((report = hid_queue_pop_report( queue )))
     {
-        memcpy( irp->AssociatedIrp.SystemBuffer, report->buffer, desc->InputLength );
+        memcpy( irp->AssociatedIrp.SystemBuffer, report->buffer, report->length );
         irp->IoStatus.Information = report->length;
         irp->IoStatus.Status = STATUS_SUCCESS;
         hid_report_decref( report );
-- 
2.33.0

[PATCH 2/3] hidclass.sys: Introduce new find_report_with_type_and_id helper.

[Rémi Bernon]

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=51828
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com>
---
 dlls/hidclass.sys/device.c | 64 ++++++++++++++++++++------------------
 1 file changed, 33 insertions(+), 31 deletions(-)

diff --git a/dlls/hidclass.sys/device.c b/dlls/hidclass.sys/device.c
index 632e98ab740..785c45dce3a 100644
--- a/dlls/hidclass.sys/device.c
+++ b/dlls/hidclass.sys/device.c
@@ -289,15 +289,30 @@ static void hid_device_queue_input( DEVICE_OBJECT *device, HID_XFER_PACKET *pack
     hid_report_decref( last_report );
 }
 
+static HIDP_REPORT_IDS *find_report_with_type_and_id( BASE_DEVICE_EXTENSION *ext, BYTE type, BYTE id, BOOL any_id )
+{
+    HIDP_REPORT_IDS *report, *reports = ext->u.pdo.device_desc.ReportIDs;
+    ULONG report_count = ext->u.pdo.device_desc.ReportIDsLength;
+
+    for (report = reports; report != reports + report_count; report++)
+    {
+        if (!any_id && report->ReportID && report->ReportID != id) continue;
+        if (type == HidP_Input && report->InputLength) return report;
+        if (type == HidP_Output && report->OutputLength) return report;
+        if (type == HidP_Feature && report->FeatureLength) return report;
+    }
+
+    return NULL;
+}
+
 static DWORD CALLBACK hid_device_thread(void *args)
 {
     DEVICE_OBJECT *device = (DEVICE_OBJECT*)args;
     BASE_DEVICE_EXTENSION *ext = device->DeviceExtension;
     HIDP_COLLECTION_DESC *desc = ext->u.pdo.device_desc.CollectionDesc;
-    HIDP_REPORT_IDS *reports = ext->u.pdo.device_desc.ReportIDs;
-    ULONG report_count = ext->u.pdo.device_desc.ReportIDsLength;
     BOOL polled = ext->u.pdo.information.Polled;
-    ULONG i, report_id = 0, timeout = 0;
+    ULONG report_id = 0, timeout = 0;
+    HIDP_REPORT_IDS *report;
     HID_XFER_PACKET *packet;
     IO_STATUS_BLOCK io;
     BYTE *buffer;
@@ -309,14 +324,9 @@ static DWORD CALLBACK hid_device_thread(void *args)
 
     if (polled) timeout = ext->u.pdo.poll_interval;
 
-    for (i = 0; i < report_count; ++i)
-    {
-        if (!reports[i].ReportID || reports[i].InputLength)
-            break;
-    }
-
-    if (i == report_count) WARN("no input report found.\n");
-    else report_id = reports[i].ReportID;
+    report = find_report_with_type_and_id( ext, HidP_Input, 0, TRUE );
+    if (!report) WARN("no input report found.\n");
+    else report_id = report->ReportID;
 
     do
     {
@@ -414,10 +424,9 @@ static void handle_minidriver_string( BASE_DEVICE_EXTENSION *ext, IRP *irp, SHOR
 
 static void hid_device_xfer_report( BASE_DEVICE_EXTENSION *ext, ULONG code, IRP *irp )
 {
-    HIDP_REPORT_IDS *reports = ext->u.pdo.device_desc.ReportIDs;
-    ULONG report_count = ext->u.pdo.device_desc.ReportIDsLength;
     IO_STACK_LOCATION *stack = IoGetCurrentIrpStackLocation( irp );
-    ULONG i, offset = 0, report_len = 0, buffer_len = 0;
+    ULONG offset = 0, report_len = 0, buffer_len = 0;
+    HIDP_REPORT_IDS *report;
     HID_XFER_PACKET packet;
     BYTE *buffer = NULL;
 
@@ -444,39 +453,32 @@ static void hid_device_xfer_report( BASE_DEVICE_EXTENSION *ext, ULONG code, IRP
         return;
     }
 
-    for (i = 0; i < report_count; ++i)
-    {
-        if (!reports[i].ReportID || reports[i].ReportID == buffer[0])
-            break;
-    }
-    if (i == report_count)
-    {
-        irp->IoStatus.Status = STATUS_INVALID_PARAMETER;
-        return;
-    }
-    if (!reports[i].ReportID) offset = 1;
-
     switch (code)
     {
     case IOCTL_HID_GET_INPUT_REPORT:
-        report_len = reports[i].InputLength;
+        report = find_report_with_type_and_id( ext, HidP_Input, buffer[0], FALSE );
+        if (report) report_len = report->InputLength;
         break;
     case IOCTL_HID_SET_OUTPUT_REPORT:
     case IOCTL_HID_WRITE_REPORT:
-        report_len = reports[i].OutputLength;
+        report = find_report_with_type_and_id( ext, HidP_Output, buffer[0], FALSE );
+        if (report) report_len = report->OutputLength;
         break;
     case IOCTL_HID_GET_FEATURE:
     case IOCTL_HID_SET_FEATURE:
-        report_len = reports[i].FeatureLength;
+        report = find_report_with_type_and_id( ext, HidP_Feature, buffer[0], FALSE );
+        if (report) report_len = report->FeatureLength;
         break;
     }
-    if (buffer_len < report_len)
+
+    if (!report || buffer_len < report_len)
     {
         irp->IoStatus.Status = STATUS_INVALID_PARAMETER;
         return;
     }
 
-    packet.reportId = reports[i].ReportID;
+    if (!report->ReportID) offset = 1;
+    packet.reportId = report->ReportID;
     packet.reportBuffer = buffer + offset;
 
     switch (code)
-- 
2.33.0

[PATCH 3/3] hidclass.sys: Drop reports when length doesn't match their declaration.

[Rémi Bernon]

Based on a patch from Ivo Ivanov <logos128(a)gmail.com>.

Instead of using the descriptor input report length, which is the
maximum length of all input reports.

Tests show that the reports should be dropped, in non-polled mode, when
their length is invalid, but we were dropping too many of them.

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=51828
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com>
---
 dlls/hidclass.sys/device.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/dlls/hidclass.sys/device.c b/dlls/hidclass.sys/device.c
index 785c45dce3a..ae8d80c2e95 100644
--- a/dlls/hidclass.sys/device.c
+++ b/dlls/hidclass.sys/device.c
@@ -349,7 +349,8 @@ static DWORD CALLBACK hid_device_thread(void *args)
             packet->reportBuffer = buffer;
             packet->reportBufferLen = io.Information;
 
-            if (polled || io.Information == desc->InputLength)
+            report = find_report_with_type_and_id( ext, HidP_Input, buffer[0], FALSE );
+            if (polled || (report && report->InputLength == io.Information))
                 hid_device_queue_input( device, packet );
         }
 
-- 
2.33.0