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. 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;
