LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@suse.de>
To: linux-usb@vger.kernel.org
Cc: linux-kernel@vger.kernel.org,
	Alan Stern <stern@rowland.harvard.edu>,
	Greg Kroah-Hartman <gregkh@suse.de>
Subject: [PATCH 5/6] USB: check serial-number string after device reset
Date: Mon, 10 Mar 2008 17:59:11 -0700	[thread overview]
Message-ID: <1205197152-18146-5-git-send-email-gregkh@suse.de> (raw)
In-Reply-To: <20080311004239.GD17890@suse.de>

From: Alan Stern <stern@rowland.harvard.edu>

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 <stern@rowland.harvard.edu>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
---
 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


  parent reply	other threads:[~2008-03-11  1:00 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-03-11  0:42 [GIT PATCH] USB suspend persistance for 2.6.25-rc5 git Greg KH
2008-03-11  0:59 ` [PATCH 1/6] USB: EHCI: carry out port handover during each root-hub resume Greg Kroah-Hartman
2008-03-11  0:59 ` [PATCH 2/6] USB: reorganize code in hub.c Greg Kroah-Hartman
2008-03-11  0:59 ` [PATCH 3/6] USB: make USB-PERSIST work after every system sleep Greg Kroah-Hartman
2008-03-11  0:59 ` [PATCH 4/6] USB: remove CONFIG_USB_PERSIST setting Greg Kroah-Hartman
2008-03-11  0:59 ` Greg Kroah-Hartman [this message]
2008-03-11  0:59 ` [PATCH 6/6] USB: enable USB-PERSIST by default Greg Kroah-Hartman
2008-03-19 17:05   ` Mark Lord
2008-03-19 21:35     ` Alan Stern
2008-03-20 14:17       ` Mark Lord
2008-03-11 10:27 ` [GIT PATCH] USB suspend persistance for 2.6.25-rc5 git Pavel Machek
2008-03-11 15:28   ` Greg KH
2008-03-11 16:18   ` Linus Torvalds

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1205197152-18146-5-git-send-email-gregkh@suse.de \
    --to=gregkh@suse.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=stern@rowland.harvard.edu \
    --subject='Re: [PATCH 5/6] USB: check serial-number string after device reset' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).