On 14.11.2016 14:35, Aric Stewart wrote:
This is a little confusing because reports to and from the device itself only have a leading reportId if the reportid is not 0. Otherwise there is no leading reportId. So we need to add a 0 reportId to reports coming from such a device and strip the leading 0 from reports written to the device.
Signed-off-by: Aric Stewart aric@codeweavers.com
dlls/hidclass.sys/descriptor.c | 31 ++++----------- dlls/hidclass.sys/device.c | 90 ++++++++++++++++++++++++++++-------------- 2 files changed, 67 insertions(+), 54 deletions(-)
0001-hidclass-All-reports-read-or-written-to-user-space-lea.txt
diff --git a/dlls/hidclass.sys/descriptor.c b/dlls/hidclass.sys/descriptor.c index 9a26f0f4..8bcda43 100644 --- a/dlls/hidclass.sys/descriptor.c +++ b/dlls/hidclass.sys/descriptor.c @@ -887,10 +887,8 @@ static WINE_HIDP_PREPARSED_DATA* build_PreparseData( new_report(wine_report, input_features[0]); data->dwInputReportCount++;
if (input_features[0]->caps.ReportID != 0)bitOffset = 8;elsebitOffset = 0;
/* Room for the reportID */bitOffset = 8; for (i = 0; i < i_count; i++) {@@ -901,10 +899,7 @@ static WINE_HIDP_PREPARSED_DATA* build_PreparseData( new_report(wine_report, input_features[i]); data->dwInputReportCount++; bitLength = max(bitOffset, bitLength);
if (input_features[i]->caps.ReportID != 0)bitOffset = 8;elsebitOffset = 0;
bitOffset = 8; } build_elements(wine_report, input_features[i], &bitOffset); count_elements(input_features[i], &data->caps.NumberInputButtonCaps,@@ -922,10 +917,7 @@ static WINE_HIDP_PREPARSED_DATA* build_PreparseData( data->dwOutputReportOffset = (BYTE*)wine_report - (BYTE*)data->InputReports; new_report(wine_report, output_features[0]); data->dwOutputReportCount++;
if (output_features[0]->caps.ReportID != 0)bitOffset = 8;elsebitOffset = 0;
bitOffset = 8; for (i = 0; i < o_count; i++) {@@ -936,10 +928,7 @@ static WINE_HIDP_PREPARSED_DATA* build_PreparseData( new_report(wine_report, output_features[i]); data->dwOutputReportCount++; bitLength = max(bitOffset, bitLength);
if (output_features[0]->caps.ReportID != 0)bitOffset = 8;elsebitOffset = 0;
bitOffset = 8; } build_elements(wine_report, output_features[i], &bitOffset); count_elements(output_features[i], &data->caps.NumberOutputButtonCaps,@@ -957,10 +946,7 @@ static WINE_HIDP_PREPARSED_DATA* build_PreparseData( data->dwFeatureReportOffset = (BYTE*)wine_report - (BYTE*)data->InputReports; new_report(wine_report, feature_features[0]); data->dwFeatureReportCount++;
if (feature_features[0]->caps.ReportID != 0)bitOffset = 8;elsebitOffset = 0;
bitOffset = 8; for (i = 0; i < f_count; i++) {@@ -971,10 +957,7 @@ static WINE_HIDP_PREPARSED_DATA* build_PreparseData( new_report(wine_report, feature_features[i]); data->dwFeatureReportCount++; bitLength = max(bitOffset, bitLength);
if (feature_features[0]->caps.ReportID != 0)bitOffset = 8;elsebitOffset = 0;
bitOffset = 8; } build_elements(wine_report, feature_features[i], &bitOffset); count_elements(feature_features[i], &data->caps.NumberFeatureButtonCaps,diff --git a/dlls/hidclass.sys/device.c b/dlls/hidclass.sys/device.c index 5dd5ebb..6454b67 100644 --- a/dlls/hidclass.sys/device.c +++ b/dlls/hidclass.sys/device.c @@ -197,6 +197,28 @@ void HID_DeleteDevice(HID_MINIDRIVER_REGISTRATION *driver, DEVICE_OBJECT *device IoDeleteDevice(device); }
+static NTSTATUS copy_packet_into_buffer(HID_XFER_PACKET *packet, BYTE* buffer, ULONG buffer_length, ULONG *out_length) +{
- *out_length = 0;
- if (buffer_length > packet->reportBufferLen)
- {
if (packet->reportId != 0){memcpy(buffer, packet->reportBuffer, packet->reportBufferLen);*out_length = packet->reportBufferLen;
The length check is a bit too strict in this case. You are returning STATUS_BUFFER_OVERFLOW even when the buffer is sufficient (buffer_length == packet->reportBufferLen).
}else{buffer[0] = 0;memcpy(&buffer[1], packet->reportBuffer, packet->reportBufferLen);*out_length = packet->reportBufferLen + 1;}return STATUS_SUCCESS;- }
- else
return STATUS_BUFFER_OVERFLOW;+}
static void HID_Device_processQueue(DEVICE_OBJECT *device) { LIST_ENTRY *entry; @@ -217,20 +239,14 @@ static void HID_Device_processQueue(DEVICE_OBJECT *device) RingBuffer_Read(ext->ring_buffer, ptr, packet, &buffer_size); if (buffer_size) {
NTSTATUS hr;
Maybe its just me, but I think "hr" is really a very bad name for NTSTATUS codes. ;)
ULONG out_length; IO_STACK_LOCATION *irpsp = IoGetCurrentIrpStackLocation(irp); packet->reportBuffer = (BYTE *)packet + sizeof(*packet); TRACE_(hid_report)("Processing Request (%i)\n",ptr);
if (irpsp->Parameters.Read.Length >= packet->reportBufferLen){memcpy(irp->AssociatedIrp.SystemBuffer, packet->reportBuffer, packet->reportBufferLen);irp->IoStatus.Information = packet->reportBufferLen;irp->IoStatus.u.Status = STATUS_SUCCESS;}else{irp->IoStatus.Information = 0;irp->IoStatus.u.Status = STATUS_BUFFER_OVERFLOW;}
hr = copy_packet_into_buffer(packet, irp->AssociatedIrp.SystemBuffer, irpsp->Parameters.Read.Length, &out_length);irp->IoStatus.u.Status = hr;irp->IoStatus.Information = out_length; } else {@@ -331,7 +347,10 @@ static DWORD CALLBACK hid_device_thread(void *args) if (!exit_now && irp->IoStatus.u.Status == STATUS_SUCCESS) { packet->reportBufferLen = irp->IoStatus.Information;
packet->reportId = packet->reportBuffer[0];
if (ext->preparseData->InputReports[0].reportID)packet->reportId = packet->reportBuffer[0];elsepacket->reportId = 0; RingBuffer_Write(ext->ring_buffer, packet); HID_Device_processQueue(device); }@@ -461,9 +480,17 @@ static NTSTATUS HID_set_to_device(DEVICE_OBJECT *device, IRP *irp) NTSTATUS rc;
TRACE_(hid_report)("Device %p Buffer length %i Buffer %p\n", device, irpsp->Parameters.DeviceIoControl.InputBufferLength, irp->AssociatedIrp.SystemBuffer);
- packet.reportBuffer = irp->AssociatedIrp.SystemBuffer;
- packet.reportId = ((char*)irp->AssociatedIrp.SystemBuffer)[0];
- packet.reportBufferLen = irpsp->Parameters.DeviceIoControl.InputBufferLength;
packet.reportId = ((BYTE*)irp->AssociatedIrp.SystemBuffer)[0];
if (packet.reportId == 0)
{
packet.reportBuffer = &((BYTE*)irp->AssociatedIrp.SystemBuffer)[1];packet.reportBufferLen = irpsp->Parameters.DeviceIoControl.InputBufferLength - 1;}
else
{
packet.reportBuffer = irp->AssociatedIrp.SystemBuffer;packet.reportBufferLen = irpsp->Parameters.DeviceIoControl.InputBufferLength;} TRACE_(hid_report)("(id %i, len %i buffer %p)\n", packet.reportId, packet.reportBufferLen, packet.reportBuffer);
rc = call_minidriver(irpsp->Parameters.DeviceIoControl.IoControlCode,
@@ -636,20 +663,15 @@ NTSTATUS WINAPI HID_Device_read(DEVICE_OBJECT *device, IRP *irp) if (buffer_size) { IO_STACK_LOCATION *irpsp = IoGetCurrentIrpStackLocation( irp );
NTSTATUS hr;
Same here.
ULONG out_length; packet->reportBuffer = (BYTE *)packet + sizeof(*packet); TRACE_(hid_report)("Got Packet %p %i\n", packet->reportBuffer, packet->reportBufferLen);
if (irpsp->Parameters.Read.Length >= packet->reportBufferLen){memcpy(irp->AssociatedIrp.SystemBuffer, packet->reportBuffer, packet->reportBufferLen);irp->IoStatus.Information = packet->reportBufferLen;irp->IoStatus.u.Status = STATUS_SUCCESS;}else{irp->IoStatus.Information = 0;irp->IoStatus.u.Status = STATUS_BUFFER_OVERFLOW;}IoCompleteRequest( irp, IO_NO_INCREMENT );
hr = copy_packet_into_buffer(packet, irp->AssociatedIrp.SystemBuffer, irpsp->Parameters.Read.Length, &out_length);irp->IoStatus.Information = out_length;irp->IoStatus.u.Status = hr;IoCompleteRequest(irp, IO_NO_INCREMENT);@@ -671,9 +693,17 @@ NTSTATUS WINAPI HID_Device_write(DEVICE_OBJECT *device, IRP *irp) irp->IoStatus.Information = 0;
TRACE_(hid_report)("Device %p Buffer length %i Buffer %p\n", device, irpsp->Parameters.Write.Length, irp->AssociatedIrp.SystemBuffer);
- packet.reportBuffer = irp->AssociatedIrp.SystemBuffer;
- packet.reportId = ((char*)irp->AssociatedIrp.SystemBuffer)[0];
- packet.reportBufferLen = irpsp->Parameters.Write.Length;
- packet.reportId = ((BYTE*)irp->AssociatedIrp.SystemBuffer)[0];
- if (packet.reportId == 0)
- {
packet.reportBuffer = &((BYTE*)irp->AssociatedIrp.SystemBuffer)[1];packet.reportBufferLen = irpsp->Parameters.Write.Length - 1;
Besides the fact that the code looks suspicious (couldn't a 0 byte also have a different meaning?) this will probably break the udev backend. As documented in https://www.kernel.org/doc/Documentation/hid/hidraw.txt, the write() command expects that the first byte is 0 if the device does not use numbered reports.
}
else
{
packet.reportBuffer = irp->AssociatedIrp.SystemBuffer;packet.reportBufferLen = irpsp->Parameters.Write.Length;} TRACE_(hid_report)("(id %i, len %i buffer %p)\n", packet.reportId, packet.reportBufferLen, packet.reportBuffer);
rc = call_minidriver(IOCTL_HID_WRITE_REPORT, device, NULL, 0, &packet, sizeof(packet));