Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757575AbYCKBAl (ORCPT ); Mon, 10 Mar 2008 21:00:41 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1756244AbYCKA7f (ORCPT ); Mon, 10 Mar 2008 20:59:35 -0400 Received: from ns2.suse.de ([195.135.220.15]:40911 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756102AbYCKA7e (ORCPT ); Mon, 10 Mar 2008 20:59:34 -0400 From: Greg Kroah-Hartman To: linux-usb@vger.kernel.org Cc: linux-kernel@vger.kernel.org, Alan Stern , Greg Kroah-Hartman Subject: [PATCH 5/6] USB: check serial-number string after device reset Date: Mon, 10 Mar 2008 17:59:11 -0700 Message-Id: <1205197152-18146-5-git-send-email-gregkh@suse.de> X-Mailer: git-send-email 1.5.4.3 In-Reply-To: <20080311004239.GD17890@suse.de> References: <20080311004239.GD17890@suse.de> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5765 Lines: 164 From: Alan Stern This patch (as1048) extends the descriptor checking after a device is reset. Now the SerialNumber string descriptor is compared to its old value, in addition to the device and configuration descriptors. As a consequence, the kmalloc() call in usb_string() is now on the error-handling pathway for usb-storage. Hence its allocation type is changed to GFO_NOIO. Signed-off-by: Alan Stern Signed-off-by: Greg Kroah-Hartman --- Documentation/usb/persist.txt | 8 ++-- drivers/usb/core/hub.c | 65 +++++++++++++++++++++++++++++++--------- drivers/usb/core/message.c | 2 +- 3 files changed, 55 insertions(+), 20 deletions(-) diff --git a/Documentation/usb/persist.txt b/Documentation/usb/persist.txt index bea58db..d56cb1a 100644 --- a/Documentation/usb/persist.txt +++ b/Documentation/usb/persist.txt @@ -136,10 +136,10 @@ aren't guaranteed to be 100% accurate. If you replace one USB device with another of the same type (same manufacturer, same IDs, and so on) there's an excellent chance the -kernel won't detect the change. Serial numbers and other strings are -not compared. In many cases it wouldn't help if they were, because -manufacturers frequently omit serial numbers entirely in their -devices. +kernel won't detect the change. The serial number string and other +descriptors are compared with the kernel's stored values, but this +might not help since manufacturers frequently omit serial numbers +entirely in their devices. Furthermore it's quite possible to leave a USB device exactly the same while changing its media. If you replace the flash memory card in a diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index 277d4ec..545fa72 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -3011,16 +3011,36 @@ void usb_hub_cleanup(void) usb_deregister(&hub_driver); } /* usb_hub_cleanup() */ -static int config_descriptors_changed(struct usb_device *udev) -{ - unsigned index; - unsigned len = 0; - struct usb_config_descriptor *buf; +static int descriptors_changed(struct usb_device *udev, + struct usb_device_descriptor *old_device_descriptor) +{ + 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; + + /* 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++) { - if (len < le16_to_cpu(udev->config[index].desc.wTotalLength)) - len = le16_to_cpu(udev->config[index].desc.wTotalLength); + old_length = le16_to_cpu(udev->config[index].desc.wTotalLength); + len = max(len, old_length); } + buf = kmalloc(len, GFP_NOIO); if (buf == NULL) { dev_err(&udev->dev, "no mem to re-read configs after reset\n"); @@ -3028,25 +3048,41 @@ static int config_descriptors_changed(struct usb_device *udev) return 1; } for (index = 0; index < udev->descriptor.bNumConfigurations; index++) { - int length; - int old_length = le16_to_cpu(udev->config[index].desc.wTotalLength); - + 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) { + 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, buf->bConfigurationValue); + 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 index != udev->descriptor.bNumConfigurations; + return changed; } /** @@ -3119,8 +3155,7 @@ int usb_reset_device(struct usb_device *udev) goto re_enumerate; /* Device might have changed firmware (DFU or similar) */ - if (memcmp(&udev->descriptor, &descriptor, sizeof descriptor) - || config_descriptors_changed (udev)) { + if (descriptors_changed(udev, &descriptor)) { dev_info(&udev->dev, "device firmware changed\n"); udev->descriptor = descriptor; /* for disconnect() calls */ goto re_enumerate; diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c index fefb922..a45076e 100644 --- a/drivers/usb/core/message.c +++ b/drivers/usb/core/message.c @@ -784,7 +784,7 @@ int usb_string(struct usb_device *dev, int index, char *buf, size_t size) if (size <= 0 || !buf || !index) return -EINVAL; buf[0] = 0; - tbuf = kmalloc(256, GFP_KERNEL); + tbuf = kmalloc(256, GFP_NOIO); if (!tbuf) return -ENOMEM; -- 1.5.4.3 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/