| Message ID | 1425266003-16279-1-git-send-email-adam.lee@canonical.com |
|---|---|
| State | New |
| Headers | show |
On Mon, Mar 02, 2015 at 11:13:23AM +0800, Adam Lee wrote: > From: Oliver Neukum <oneukum@suse.de> > > BugLink: http://bugs.launchpad.net/bugs/1413992 > > Check the special CDC headers for a plausible minimum length. > Another big operating systems ignores such garbage. > > Signed-off-by: Oliver Neukum <oneukum@suse.de> > CC: stable@vger.kernel.org > Reviewed-by: Adam Lee <adam8157@gmail.com> > Tested-by: Adam Lee <adam8157@gmail.com> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > (cherry picked from commit 7e860a6e7aa62b337a61110430cd633db5b0d2dd) > Signed-off-by: Adam Lee <adam.lee@canonical.com> > --- > drivers/usb/class/cdc-acm.c | 21 ++++++++++++++++----- > 1 file changed, 16 insertions(+), 5 deletions(-) > > diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c > index 6746103..82dea40 100644 > --- a/drivers/usb/class/cdc-acm.c > +++ b/drivers/usb/class/cdc-acm.c > @@ -943,6 +943,7 @@ static int acm_probe(struct usb_interface *intf, > unsigned long quirks; > int num_rx_buf; > int i; > + unsigned int elength = 0; > int combined_interfaces = 0; > struct device *tty_dev; > int rv = -ENOMEM; > @@ -988,9 +989,12 @@ static int acm_probe(struct usb_interface *intf, > dev_err(&intf->dev, "skipping garbage\n"); > goto next_desc; > } > + elength = buffer[0]; > > switch (buffer[2]) { > case USB_CDC_UNION_TYPE: /* we've found it */ > + if (elength < sizeof(struct usb_cdc_union_desc)) > + goto next_desc; > if (union_header) { > dev_err(&intf->dev, "More than one " > "union descriptor, skipping ...\n"); > @@ -999,31 +1003,38 @@ static int acm_probe(struct usb_interface *intf, > union_header = (struct usb_cdc_union_desc *)buffer; > break; > case USB_CDC_COUNTRY_TYPE: /* export through sysfs*/ > + if (elength < sizeof(struct usb_cdc_country_functional_desc)) > + goto next_desc; > cfd = (struct usb_cdc_country_functional_desc *)buffer; > break; > case USB_CDC_HEADER_TYPE: /* maybe check version */ > break; /* for now we ignore it */ > case USB_CDC_ACM_TYPE: > + if (elength < 4) > + goto next_desc; > ac_management_function = buffer[3]; > break; > case USB_CDC_CALL_MANAGEMENT_TYPE: > + if (elength < 5) > + goto next_desc; > call_management_function = buffer[3]; > call_interface_num = buffer[4]; > if ((quirks & NOT_A_MODEM) == 0 && (call_management_function & 3) != 3) > dev_err(&intf->dev, "This device cannot do calls on its own. It is not a modem.\n"); > break; > default: > - /* there are LOTS more CDC descriptors that > + /* > + * there are LOTS more CDC descriptors that > * could legitimately be found here. > */ > dev_dbg(&intf->dev, "Ignoring descriptor: " > - "type %02x, length %d\n", > - buffer[2], buffer[0]); > + "type %02x, length %ud\n", > + buffer[2], elength); > break; > } > next_desc: > - buflen -= buffer[0]; > - buffer += buffer[0]; > + buflen -= elength; > + buffer += elength; > } > > if (!union_header) { Looks to do what is claimed, and bug parity with the "other operating systems" is a laudible goal. Acked-by: Andy Whitcroft <apw@canonical.com> -apw
Applied to vivid master-next. Note that I had to re-cherry-pick this as the one included here did not apply (oddness). -apw
This is looking for another review. -apw
Appears plausible. But also is probably (or should be) part of the stable trees...
Applied to trusty and utopic -apw
diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c index 6746103..82dea40 100644 --- a/drivers/usb/class/cdc-acm.c +++ b/drivers/usb/class/cdc-acm.c @@ -943,6 +943,7 @@ static int acm_probe(struct usb_interface *intf, unsigned long quirks; int num_rx_buf; int i; + unsigned int elength = 0; int combined_interfaces = 0; struct device *tty_dev; int rv = -ENOMEM; @@ -988,9 +989,12 @@ static int acm_probe(struct usb_interface *intf, dev_err(&intf->dev, "skipping garbage\n"); goto next_desc; } + elength = buffer[0]; switch (buffer[2]) { case USB_CDC_UNION_TYPE: /* we've found it */ + if (elength < sizeof(struct usb_cdc_union_desc)) + goto next_desc; if (union_header) { dev_err(&intf->dev, "More than one " "union descriptor, skipping ...\n"); @@ -999,31 +1003,38 @@ static int acm_probe(struct usb_interface *intf, union_header = (struct usb_cdc_union_desc *)buffer; break; case USB_CDC_COUNTRY_TYPE: /* export through sysfs*/ + if (elength < sizeof(struct usb_cdc_country_functional_desc)) + goto next_desc; cfd = (struct usb_cdc_country_functional_desc *)buffer; break; case USB_CDC_HEADER_TYPE: /* maybe check version */ break; /* for now we ignore it */ case USB_CDC_ACM_TYPE: + if (elength < 4) + goto next_desc; ac_management_function = buffer[3]; break; case USB_CDC_CALL_MANAGEMENT_TYPE: + if (elength < 5) + goto next_desc; call_management_function = buffer[3]; call_interface_num = buffer[4]; if ((quirks & NOT_A_MODEM) == 0 && (call_management_function & 3) != 3) dev_err(&intf->dev, "This device cannot do calls on its own. It is not a modem.\n"); break; default: - /* there are LOTS more CDC descriptors that + /* + * there are LOTS more CDC descriptors that * could legitimately be found here. */ dev_dbg(&intf->dev, "Ignoring descriptor: " - "type %02x, length %d\n", - buffer[2], buffer[0]); + "type %02x, length %ud\n", + buffer[2], elength); break; } next_desc: - buflen -= buffer[0]; - buffer += buffer[0]; + buflen -= elength; + buffer += elength; } if (!union_header) {