On Fri, 29 Jan 2016, Dan Carpenter wrote:
> Hello Bryan Wu,
>
> 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
> 358 static void f_audio_complete(struct usb_ep *ep, struct usb_request *req)
> 359 {
> 360 struct f_audio *audio = req->context;
> 361 int status = req->status;
> 362 u32 data = 0;
> ^^^^^^^^
> data is a u32.
>
> 363 struct usb_ep *out_ep = audio->out_ep;
> 364
> 365 switch (status) {
> 366
> 367 case 0: /* normal completion? */
> 368 if (ep == out_ep)
> 369 f_audio_out_ep_complete(ep, req);
> 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.
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
