kevin/linux
1
Fork 0
Code

USB: core: Fix duplicate endpoint bug by clearing reserved bits in the descriptor

Browse Source 
Syzbot has identified a bug in usbcore (see the Closes: tag below)
caused by our assumption that the reserved bits in an endpoint
descriptor's bEndpointAddress field will always be 0.  As a result of
the bug, the endpoint_is_duplicate() routine in config.c (and possibly
other routines as well) may believe that two descriptors are for
distinct endpoints, even though they have the same direction and
endpoint number.  This can lead to confusion, including the bug
identified by syzbot (two descriptors with matching endpoint numbers
and directions, where one was interrupt and the other was bulk).

To fix the bug, we will clear the reserved bits in bEndpointAddress
when we parse the descriptor.  (Note that both the USB-2.0 and USB-3.1
specs say these bits are "Reserved, reset to zero".)  This requires us
to make a copy of the descriptor earlier in usb_parse_endpoint() and
use the copy instead of the original when checking for duplicates.

Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Reported-and-tested-by: syzbot+8693a0bb9c10b554272a@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/linux-usb/0000000000003d868e061bc0f554@google.com/
Fixes: 0a8fd13462 ("USB: fix problems with duplicate endpoint addresses")
CC: Oliver Neukum <oneukum@suse.com>
CC: stable@vger.kernel.org
Link: https://lore.kernel.org/r/205a5edc-7fef-4159-b64a-80374b6b101a@rowland.harvard.edu
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This commit is contained in:
Alan Stern 2024-06-27 15:56:18 -04:00 committed by Greg Kroah-Hartman
parent 79989bd4ab
commit a368ecde8a
1 changed files with 15 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
drivers/usb/core/config.c
View File

@ -291,6 +291,20 @@ static int usb_parse_endpoint(struct device *ddev, int cfgno,
if (ifp->desc.bNumEndpoints >= num_ep)
goto skip_to_next_endpoint_or_interface_descriptor;
/* Save a copy of the descriptor and use it instead of the original */
endpoint = &ifp->endpoint[ifp->desc.bNumEndpoints];
memcpy(&endpoint->desc, d, n);
d = &endpoint->desc;
/* Clear the reserved bits in bEndpointAddress */
i = d->bEndpointAddress &
(USB_ENDPOINT_DIR_MASK | USB_ENDPOINT_NUMBER_MASK);
if (i != d->bEndpointAddress) {
dev_notice(ddev, "config %d interface %d altsetting %d has an endpoint descriptor with address 0x%X, changing to 0x%X\n",
cfgno, inum, asnum, d->bEndpointAddress, i);
endpoint->desc.bEndpointAddress = i;
}
/* Check for duplicate endpoint addresses */
if (config_endpoint_is_duplicate(config, inum, asnum, d)) {
dev_notice(ddev, "config %d interface %d altsetting %d has a duplicate endpoint with address 0x%X, skipping\n",
@ -308,10 +322,8 @@ static int usb_parse_endpoint(struct device *ddev, int cfgno,
}
}
endpoint = &ifp->endpoint[ifp->desc.bNumEndpoints];
/* Accept this endpoint */
++ifp->desc.bNumEndpoints;
memcpy(&endpoint->desc, d, n);
INIT_LIST_HEAD(&endpoint->urb_list);
/*