On Mon, Oct 21, 2013 at 11:57:32AM -0400, Alan Stern wrote:
> On Mon, 21 Oct 2013, Huang Rui wrote:
>
> > + /*
> > + * get generic device-level capability descriptors [9.6.2]
> > + * in USB 3.0 spec
> > + */
> > + retval = usb_get_descriptor(udev, USB_DT_BOS, 0, dev->buf,
> > + total);
>
> This exposes the kernel to a buffer overflow bug. Remember, dev->buf
> is only 256 bytes long. What happens if total > 256?
>
Do you mean I should allocate a buffer with "total" size? Or if
"total" > 256, I set a dev_err then return?
A question, I think "total" doesn't larger than 256. Because at
current, there are only four device capability types such as
Wireless_USB, USB 2.0 EXETENSION, Superspeed_USB, CONTAINER_ID, do
you mean there might be more desciptors added in future?
> > + if (retval != total) {
> > + dev_err(&iface->dev, "bos descriptor set --> %d\n",
> > + retval);
> > + return (retval < 0) ? retval : -EDOM;
> > + }
> > +
> > + length = sizeof(*udev->bos->desc);
> > + for (i = 0; i < num; i++) {
> > + dev->buf += length;
>
> What will happen when the code further down uses dev->buf to hold
> config descriptors? You should use a local pointer here; don't change
> dev->buf.
>
Right, will update.
> What happens if the new value of the pointer lies beyond the end of the
> dev->buf?
>
Got it, will update.
> > + header = (struct usb_dev_cap_header *)dev->buf;
> > + length = header->bLength;
> > +
> > + if (header->bDescriptorType !=
> > + USB_DT_DEVICE_CAPABILITY) {
> > + dev_warn(&udev->dev, "not device capability
> > descriptor, skip\n");
> > + continue;
> > + }
> > +
> > + switch (header->bDevCapabilityType) {
> > + case USB_CAP_TYPE_EXT:
> > + if (!is_good_ext(dev)) {
>
> What happens if header points to a location only 1 or 2 bytes before
> the end of dev->buf?
>
Will update.
> > + dev_err(&iface->dev, "bogus usb 2.0
> > extension descriptor\n");
> > + return -EDOM;
> > + }
> > + break;
> > + default:
> > + break;
> > + }
> > + }
> > }
>
Thank you to your comments.
Thanks,
Rui
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
