usb: hub: Check device descriptor before resusciation

If a device connected to an xHCI host controller disconnects from the USB bus
and then reconnects, e.g. triggered by a firmware update, then the host
controller automatically activates the connection and the port is enabled. The
implementation of hub_port_connect_change() assumes that if the port is
enabled then nothing has changed. There is no check if the USB descriptors
have changed. As a result, the kernel's internal copy of the descriptors ends
up being incorrect and the device doesn't work properly anymore.

The solution to the problem is for hub_port_connect_change() always to
check whether the device's descriptors have changed before resuscitating
an enabled port.

Signed-off-by: David Heinzelmann <heinzelmann.david@gmail.com>
Acked-by: Alan Stern <stern@rowland.harvard.edu>
Link: https://lore.kernel.org/r/20191009044647.24536-1-heinzelmann.david@gmail.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This commit is contained in:
David Heinzelmann 2019-10-09 06:46:47 +02:00 committed by Greg Kroah-Hartman
parent db0386312f
commit a4f55d8b8c

View File

@ -4930,6 +4930,91 @@ hub_power_remaining(struct usb_hub *hub)
return remaining;
}
static int descriptors_changed(struct usb_device *udev,
struct usb_device_descriptor *old_device_descriptor,
struct usb_host_bos *old_bos)
{
int changed = 0;
unsigned index;
unsigned serial_len = 0;
unsigned len;
unsigned old_length;
int length;
char *buf;
if (memcmp(&udev->descriptor, old_device_descriptor,
sizeof(*old_device_descriptor)) != 0)
return 1;
if ((old_bos && !udev->bos) || (!old_bos && udev->bos))
return 1;
if (udev->bos) {
len = le16_to_cpu(udev->bos->desc->wTotalLength);
if (len != le16_to_cpu(old_bos->desc->wTotalLength))
return 1;
if (memcmp(udev->bos->desc, old_bos->desc, len))
return 1;
}
/* Since the idVendor, idProduct, and bcdDevice values in the
* device descriptor haven't changed, we will assume the
* Manufacturer and Product strings haven't changed either.
* But the SerialNumber string could be different (e.g., a
* different flash card of the same brand).
*/
if (udev->serial)
serial_len = strlen(udev->serial) + 1;
len = serial_len;
for (index = 0; index < udev->descriptor.bNumConfigurations; index++) {
old_length = le16_to_cpu(udev->config[index].desc.wTotalLength);
len = max(len, old_length);
}
buf = kmalloc(len, GFP_NOIO);
if (!buf)
/* assume the worst */
return 1;
for (index = 0; index < udev->descriptor.bNumConfigurations; index++) {
old_length = le16_to_cpu(udev->config[index].desc.wTotalLength);
length = usb_get_descriptor(udev, USB_DT_CONFIG, index, buf,
old_length);
if (length != old_length) {
dev_dbg(&udev->dev, "config index %d, error %d\n",
index, length);
changed = 1;
break;
}
if (memcmp(buf, udev->rawdescriptors[index], old_length)
!= 0) {
dev_dbg(&udev->dev, "config index %d changed (#%d)\n",
index,
((struct usb_config_descriptor *) buf)->
bConfigurationValue);
changed = 1;
break;
}
}
if (!changed && serial_len) {
length = usb_string(udev, udev->descriptor.iSerialNumber,
buf, serial_len);
if (length + 1 != serial_len) {
dev_dbg(&udev->dev, "serial string error %d\n",
length);
changed = 1;
} else if (memcmp(buf, udev->serial, length) != 0) {
dev_dbg(&udev->dev, "serial string changed\n");
changed = 1;
}
}
kfree(buf);
return changed;
}
static void hub_port_connect(struct usb_hub *hub, int port1, u16 portstatus,
u16 portchange)
{
@ -5167,7 +5252,9 @@ static void hub_port_connect_change(struct usb_hub *hub, int port1,
{
struct usb_port *port_dev = hub->ports[port1 - 1];
struct usb_device *udev = port_dev->child;
struct usb_device_descriptor descriptor;
int status = -ENODEV;
int retval;
dev_dbg(&port_dev->dev, "status %04x, change %04x, %s\n", portstatus,
portchange, portspeed(hub, portstatus));
@ -5188,7 +5275,30 @@ static void hub_port_connect_change(struct usb_hub *hub, int port1,
if ((portstatus & USB_PORT_STAT_CONNECTION) && udev &&
udev->state != USB_STATE_NOTATTACHED) {
if (portstatus & USB_PORT_STAT_ENABLE) {
status = 0; /* Nothing to do */
/*
* USB-3 connections are initialized automatically by
* the hostcontroller hardware. Therefore check for
* changed device descriptors before resuscitating the
* device.
*/
descriptor = udev->descriptor;
retval = usb_get_device_descriptor(udev,
sizeof(udev->descriptor));
if (retval < 0) {
dev_dbg(&udev->dev,
"can't read device descriptor %d\n",
retval);
} else {
if (descriptors_changed(udev, &descriptor,
udev->bos)) {
dev_dbg(&udev->dev,
"device descriptor has changed\n");
/* for disconnect() calls */
udev->descriptor = descriptor;
} else {
status = 0; /* Nothing to do */
}
}
#ifdef CONFIG_PM
} else if (udev->state == USB_STATE_SUSPENDED &&
udev->persist_enabled) {
@ -5550,90 +5660,6 @@ void usb_hub_cleanup(void)
usb_deregister(&hub_driver);
} /* usb_hub_cleanup() */
static int descriptors_changed(struct usb_device *udev,
struct usb_device_descriptor *old_device_descriptor,
struct usb_host_bos *old_bos)
{
int changed = 0;
unsigned index;
unsigned serial_len = 0;
unsigned len;
unsigned old_length;
int length;
char *buf;
if (memcmp(&udev->descriptor, old_device_descriptor,
sizeof(*old_device_descriptor)) != 0)
return 1;
if ((old_bos && !udev->bos) || (!old_bos && udev->bos))
return 1;
if (udev->bos) {
len = le16_to_cpu(udev->bos->desc->wTotalLength);
if (len != le16_to_cpu(old_bos->desc->wTotalLength))
return 1;
if (memcmp(udev->bos->desc, old_bos->desc, len))
return 1;
}
/* Since the idVendor, idProduct, and bcdDevice values in the
* device descriptor haven't changed, we will assume the
* Manufacturer and Product strings haven't changed either.
* But the SerialNumber string could be different (e.g., a
* different flash card of the same brand).
*/
if (udev->serial)
serial_len = strlen(udev->serial) + 1;
len = serial_len;
for (index = 0; index < udev->descriptor.bNumConfigurations; index++) {
old_length = le16_to_cpu(udev->config[index].desc.wTotalLength);
len = max(len, old_length);
}
buf = kmalloc(len, GFP_NOIO);
if (!buf)
/* assume the worst */
return 1;
for (index = 0; index < udev->descriptor.bNumConfigurations; index++) {
old_length = le16_to_cpu(udev->config[index].desc.wTotalLength);
length = usb_get_descriptor(udev, USB_DT_CONFIG, index, buf,
old_length);
if (length != old_length) {
dev_dbg(&udev->dev, "config index %d, error %d\n",
index, length);
changed = 1;
break;
}
if (memcmp(buf, udev->rawdescriptors[index], old_length)
!= 0) {
dev_dbg(&udev->dev, "config index %d changed (#%d)\n",
index,
((struct usb_config_descriptor *) buf)->
bConfigurationValue);
changed = 1;
break;
}
}
if (!changed && serial_len) {
length = usb_string(udev, udev->descriptor.iSerialNumber,
buf, serial_len);
if (length + 1 != serial_len) {
dev_dbg(&udev->dev, "serial string error %d\n",
length);
changed = 1;
} else if (memcmp(buf, udev->serial, length) != 0) {
dev_dbg(&udev->dev, "serial string changed\n");
changed = 1;
}
}
kfree(buf);
return changed;
}
/**
* usb_reset_and_verify_device - perform a USB port reset to reinitialize a device
* @udev: device to reset (not in SUSPENDED or NOTATTACHED state)