[PATCH] hidclass.sys: fixed size passed IOCTL_HID_GET_INPUT_REPORT (Coverity)
Aric Stewart
aric at codeweavers.com
Mon Oct 19 08:04:37 CDT 2015
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 at 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
More information about the wine-devel
mailing list