On Wed, Sep 4, 2019 at 4:41 PM Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote: > > On Tue, 3 Sep 2019, syzbot wrote: > > > Hello, > > > > syzbot has tested the proposed patch but the reproducer still triggered > > crash: > > KASAN: slab-out-of-bounds Read in usb_reset_and_verify_device > > > > usb 6-1: Using ep0 maxpacket: 16 > > usb 6-1: BOS total length 54, descriptor 168 > > usb 6-1: Old BOS ffff8881cd814f60 Len 0xa8 > > usb 6-1: New BOS ffff8881cd257ae0 Len 0xa8 > > ================================================================== > > BUG: KASAN: slab-out-of-bounds in memcmp+0xa6/0xb0 lib/string.c:904 > > Read of size 1 at addr ffff8881cd257c36 by task kworker/1:0/17 > > Very sneaky! A BOS descriptor whose wTotalLength field varies > depending on how many bytes you read. > > This should fix it. It's the same approach we use for the Config > descriptor. Nice, core USB bug :) Can this potentially lead to something worse than a out-of-bounds memcmp? > > Alan Stern > > #syz test: https://github.com/google/kasan.git eea39f24 > > drivers/usb/core/config.c | 12 ++++++++---- > 1 file changed, 8 insertions(+), 4 deletions(-) > > Index: usb-devel/drivers/usb/core/config.c > =================================================================== > --- usb-devel.orig/drivers/usb/core/config.c > +++ usb-devel/drivers/usb/core/config.c > @@ -921,7 +921,7 @@ int usb_get_bos_descriptor(struct usb_de > struct usb_bos_descriptor *bos; > struct usb_dev_cap_header *cap; > struct usb_ssp_cap_descriptor *ssp_cap; > - unsigned char *buffer; > + unsigned char *buffer, *buffer0; > int length, total_len, num, i, ssac; > __u8 cap_type; > int ret; > @@ -966,10 +966,12 @@ int usb_get_bos_descriptor(struct usb_de > ret = -ENOMSG; > goto err; > } > + > + buffer0 = buffer; > total_len -= length; > + buffer += length; > > for (i = 0; i < num; i++) { > - buffer += length; > cap = (struct usb_dev_cap_header *)buffer; > > if (total_len < sizeof(*cap) || total_len < cap->bLength) { > @@ -983,8 +985,6 @@ int usb_get_bos_descriptor(struct usb_de > break; > } > > - total_len -= length; > - > if (cap->bDescriptorType != USB_DT_DEVICE_CAPABILITY) { > dev_warn(ddev, "descriptor type invalid, skip\n"); > continue; > @@ -1019,7 +1019,11 @@ int usb_get_bos_descriptor(struct usb_de > default: > break; > } > + > + total_len -= length; > + buffer += length; > } > + dev->bos->desc->wTotalLength = cpu_to_le16(buffer - buffer0); > > return 0; > >
