Skip to content

Commit 77ce180

Browse files
AlanSternbwhacks
authored andcommitted
USB: usbcore: Fix slab-out-of-bounds bug during device reset
commit 3dd550a upstream. The syzbot fuzzer provoked a slab-out-of-bounds error in the USB core: BUG: KASAN: slab-out-of-bounds in memcmp+0xa6/0xb0 lib/string.c:904 Read of size 1 at addr ffff8881d175bed6 by task kworker/0:3/2746 CPU: 0 PID: 2746 Comm: kworker/0:3 Not tainted 5.3.0-rc5+ #28 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 Workqueue: usb_hub_wq hub_event Call Trace: __dump_stack lib/dump_stack.c:77 [inline] dump_stack+0xca/0x13e lib/dump_stack.c:113 print_address_description+0x6a/0x32c mm/kasan/report.c:351 __kasan_report.cold+0x1a/0x33 mm/kasan/report.c:482 kasan_report+0xe/0x12 mm/kasan/common.c:612 memcmp+0xa6/0xb0 lib/string.c:904 memcmp include/linux/string.h:400 [inline] descriptors_changed drivers/usb/core/hub.c:5579 [inline] usb_reset_and_verify_device+0x564/0x1300 drivers/usb/core/hub.c:5729 usb_reset_device+0x4c1/0x920 drivers/usb/core/hub.c:5898 rt2x00usb_probe+0x53/0x7af drivers/net/wireless/ralink/rt2x00/rt2x00usb.c:806 The error occurs when the descriptors_changed() routine (called during a device reset) attempts to compare the old and new BOS and capability descriptors. The length it uses for the comparison is the wTotalLength value stored in BOS descriptor, but this value is not necessarily the same as the length actually allocated for the descriptors. If it is larger the routine will call memcmp() with a length that is too big, thus reading beyond the end of the allocated region and leading to this fault. The kernel reads the BOS descriptor twice: first to get the total length of all the capability descriptors, and second to read it along with all those other descriptors. A malicious (or very faulty) device may send different values for the BOS descriptor fields each time. The memory area will be allocated using the wTotalLength value read the first time, but stored within it will be the value read the second time. To prevent this possibility from causing any errors, this patch modifies the BOS descriptor after it has been read the second time: It sets the wTotalLength field to the actual length of the descriptors that were read in and validated. Then the memcpy() call, or any other code using these descriptors, will be able to rely on wTotalLength being valid. Reported-and-tested-by: [email protected] Signed-off-by: Alan Stern <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Greg Kroah-Hartman <[email protected]> Signed-off-by: Ben Hutchings <[email protected]>
1 parent bb459b7 commit 77ce180

File tree

1 file changed

+8
-4
lines changed

1 file changed

+8
-4
lines changed

drivers/usb/core/config.c

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -886,7 +886,7 @@ int usb_get_bos_descriptor(struct usb_device *dev)
886886
struct device *ddev = &dev->dev;
887887
struct usb_bos_descriptor *bos;
888888
struct usb_dev_cap_header *cap;
889-
unsigned char *buffer;
889+
unsigned char *buffer, *buffer0;
890890
int length, total_len, num, i;
891891
__u8 cap_type;
892892
int ret;
@@ -931,10 +931,12 @@ int usb_get_bos_descriptor(struct usb_device *dev)
931931
ret = -ENOMSG;
932932
goto err;
933933
}
934+
935+
buffer0 = buffer;
934936
total_len -= length;
937+
buffer += length;
935938

936939
for (i = 0; i < num; i++) {
937-
buffer += length;
938940
cap = (struct usb_dev_cap_header *)buffer;
939941

940942
if (total_len < sizeof(*cap) || total_len < cap->bLength) {
@@ -948,8 +950,6 @@ int usb_get_bos_descriptor(struct usb_device *dev)
948950
break;
949951
}
950952

951-
total_len -= length;
952-
953953
if (cap->bDescriptorType != USB_DT_DEVICE_CAPABILITY) {
954954
dev_warn(ddev, "descriptor type invalid, skip\n");
955955
continue;
@@ -974,7 +974,11 @@ int usb_get_bos_descriptor(struct usb_device *dev)
974974
default:
975975
break;
976976
}
977+
978+
total_len -= length;
979+
buffer += length;
977980
}
981+
dev->bos->desc->wTotalLength = cpu_to_le16(buffer - buffer0);
978982

979983
return 0;
980984

0 commit comments

Comments
 (0)