Alan Stern wrote:
> On Sun, 31 Jan 2016, Clemens Ladisch wrote:
>> Alan Stern wrote:
>>> On Fri, 29 Jan 2016, Dan Carpenter wrote:
>>>> The patch c6994e6f067c: "USB: gadget: add USB Audio Gadget driver"
>>>> from Jun 3, 2009, leads to the following static checker warning:
>>>>
>>>> drivers/usb/gadget/function/f_uac1.c:371 f_audio_complete()
>>>> error: __memcpy() '&data' too small (4 vs u32max)
>>>>
>>>> drivers/usb/gadget/function/f_uac1.c
>>>> 370 else if (audio->set_con) {
>>>> 371 memcpy(&data, req->buf, req->length);
>>>> ^^^^^^^^^^^
>>>> Back in 2014 Kees ran a USB fuzzer on the kernel. My take on that was
>>>> that req->length could not be trusted and had to be capped so I changed
>>>> Smatch to complain about these. But after a while I decided I was not
>>>> sure enough of the rules so I just ignore those bugs these days... What
>>>> are the rules?
>>>
>>> req->length is set by the driver and not changed anywhere else in the
>>> kernel. (Or rather, changing it would be a bug.) Of course, it's
>>> possible that the driver could be fooled into setting req->length to
>>> something larger than 4 -- I haven't looked through the code.
>>
>> That value comes from the control setup packet's wLength field. Neither
>> audio_set_intf_req() nor f_audio_setup() check it.
>
> Maybe those routines don't check wLength, but it has to get set
> somewhere in the driver.
In a USB gadget driver, these packets are received from the host PC.
Which, for the purposes of this discussion, must be considered evil.
Regards,
Clemens
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
