On Mon, Sep 10, 2012 at 04:45:06PM +0200, Sebastian Andrzej Siewior wrote:
> On Mon, Sep 10, 2012 at 05:21:39PM +0300, Dan Carpenter wrote:
> > Hello Sebastian Andrzej Siewior,
> >
> > The patch 3b9c1c5ba7a95: "usb: gadget: dummy_hcd: add support for
> > USB_DT_BOS on rh" from Oct 19, 2012, leads to the following warning:
> > drivers/usb/gadget/dummy_hcd.c:2038 dummy_hub_control()
> > error: memcpy() 'buf' too small (15 vs 16)
> >
> > 2031 case DeviceRequest | USB_REQ_GET_DESCRIPTOR:
> > 2032 if (hcd->speed != HCD_USB3)
> > 2033 goto error;
> > 2034
> > 2035 if ((wValue >> 8) != USB_DT_BOS)
> > 2036 goto error;
> > 2037
> > 2038 memcpy(buf, &usb3_bos_desc, sizeof(usb3_bos_desc));
> > ^^^^^^^^^^^^^^^^^^^^^^
> > 2039 retval = sizeof(usb3_bos_desc);
> > 2040 break;
> >
> >
> > buf is declared in rh_call_control() as:
> > u8 tbuf[USB_DT_BOS_SIZE + USB_DT_USB_SS_CAP_SIZE]
> > __attribute__((aligned(4)));
>
> What kind of sorcery is this? This sorcery is off by one.
>
> > The new code might work because of the alignment, but it's not
> > beautiful.
>
> Both structs are defined as packed. The first struct (usb_bos_descriptor) has
> 5 bytes, the second (usb_ss_cap_descriptor) has the bytes. This leads us to
> the following equation:
> 5 + 10 = 15
Ah crap... This is a bug in Sparse (which Smatch uses as a parser).
It doesn't handle packed structs correctly. Sorry for the noise.
>
> Regadging beautiful: Alan introduced it in ("USB: Revised fixups for root-hub
> message handler") [0]. Sarah extended this to 15 bytes for USB3 that means
> xhci code is also on the edge. I'm innocent here. You can only blame me for
> looking the other way while adding BOS descriptor to dummy.
>
I meant something else. I thought that sizeof(usb3_bos_desc) was
16 but that it still would work in testing.
regards,
dan carpenter
--
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
