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. The routine that sets wLength should make
sure that it can never be larger than 4.
Dan asked about the rules for these things. The rule is that the
driver is completely responsible for setting both the wLength and
length fields. Nothing else in the kernel is supposed to touch them.
Alan Stern
--
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
