On 10/19/15 9:08 AM, Sebastian Lackner wrote:
On 19.10.2015 15:04, Aric Stewart wrote:
On 10/17/15 8:34 AM, Marcus Meissner wrote:
On Sat, Oct 17, 2015 at 03:31:24PM +0200, Sebastian Lackner wrote:
On 17.10.2015 14:52, Marcus Meissner wrote:
1327477 Wrong sizeof argument
Signed-off-by: Marcus Meissner marcus@jet.franken.de
dlls/hidclass.sys/device.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/dlls/hidclass.sys/device.c b/dlls/hidclass.sys/device.c index e7e7e11..dcc7d3c 100644 --- a/dlls/hidclass.sys/device.c +++ b/dlls/hidclass.sys/device.c @@ -277,7 +277,7 @@ static DWORD CALLBACK hid_device_thread(void *args) packet->reportId = 0;
irp = IoBuildDeviceIoControlRequest(IOCTL_HID_GET_INPUT_REPORT,
device, NULL, 0, packet, sizeof(packet), TRUE, events[0],
device, NULL, 0, packet, sizeof(*packet)+ext->preparseData->caps.InputReportByteLength, TRUE, events[0], &irp_status); irpsp = IoGetNextIrpStackLocation(irp);This looks wrong, you have to allocate a separate buffer, and then copy it (like in the code below). I don't know why some of these buffers are HEAP_ZERO_MEMORY though, and others not? Adding Aric, he might want to review this part again, especially since Coverity detected more issues in this code (unnecessary assignment of "rc" for example).
something seems wrong though here, yes.
rc issue is "CID 1327478 Unused value"
Ciao, Marcus
What is happening here is that the HID_XFER_PACKET is not a variable length structure, it takes a pointer to the buffer. I am taking a bit of a shortcut and instead of allocating the HID_XFER_PACKET structure and a separate buffer, I am allocating 1 block of memory that is both, the buffer just comes after the HID_XFER_PACKET structure.
I am pretty sure Marcus' original patch is unnecessary. I am not surprised that there are issues in this code area though. It when through a lot of revision and tweaking as I was getting things working and that often results in cruft.
-aric
I still don't think its correct. The IOCTL_HID_GET_INPUT_REPORT expects a raw data buffer, without any header in front of it, right? Then you would have to pass packet->reportBuffer (and the correct size) to IoBuildDeviceIoControlRequest, but that doesn't work because the buffer will be deallocated automatically after the request is done.
The confusion here, and I remember it well, is that IOCTL_HID_GET_INPUT_REPORT is used twice, one from user->hidclass IOCTL and again from hidclass->minidriver IOCTL, and the IOCTL parameters are different depending on usage.
This case is the minidriver IOCTL: https://msdn.microsoft.com/en-us/library/windows/hardware/ff541126%28v=vs.85...
The minidriver IOCTL uses the HID_XFER_PACKET structure.
-aric