ChangeSet 1.1003.36.4, 2003/07/15 15:21:39-07:00, henning@meier-geinitz.de

[PATCH] USB: Fix crash when scanners are disconnected while open

This patch fixes the crash that occurs in the scanner driver when the
device is disconnected while it's still open and an application tries
to call write/read/ioctl. The patch is from Sergey Vlasov. It's the
simplest approach I've seen so far (without porting kobject to 2.4). I
can't get the scanner driver crashing with the patch applied even when
I tried hard.


 drivers/usb/scanner.c |   71 +++++++++++++++++++++++++++++++++++---------------
 1 files changed, 51 insertions(+), 20 deletions(-)


diff -Nru a/drivers/usb/scanner.c b/drivers/usb/scanner.c
--- a/drivers/usb/scanner.c	Thu Aug 28 14:50:37 2003
+++ b/drivers/usb/scanner.c	Thu Aug 28 14:50:37 2003
@@ -372,6 +372,10 @@
  *      scanners.
  *    - When checking if all minors are used don't read beyond p_scn_table
  *      (Sergey Vlasov).
+ *    - Kfree the scn structure only after disconnect AND close have occured and
+ *      check for scn->present.  This avoids crashing when someone writes (reads) to 
+ *      the device while it's already disconnected but still open. Patch from
+ *      Sergey Vlasov.
  *
  * TODO
  *    - Performance
@@ -417,6 +421,8 @@
  */ 
 #include "scanner.h"
 
+static void purge_scanner(struct scn_usb_data *scn);
+
 static void
 irq_scanner(struct urb *urb)
 {
@@ -507,28 +513,20 @@
 static int
 close_scanner(struct inode * inode, struct file * file)
 {
-	struct scn_usb_data *scn;
-
-	kdev_t scn_minor;
+	struct scn_usb_data *scn = file->private_data;
 
-	scn_minor = USB_SCN_MINOR (inode);
-
-	dbg("close_scanner: scn_minor:%d", scn_minor);
-
-	if (!p_scn_table[scn_minor]) {
-		err("close_scanner(%d): invalid scn_minor", scn_minor);
-		return -ENODEV;
-	}
-
-	down(&scn_mutex);
-
-	scn = p_scn_table[scn_minor];
 	down(&(scn->sem));
 	scn->isopen = 0;
 
 	file->private_data = NULL;
 
-	up(&scn_mutex);
+	if (!scn->present) {
+		/* The device was unplugged while open - need to clean up */
+		up(&(scn->sem));
+		purge_scanner(scn);
+		return 0;
+	}
+
 	up(&(scn->sem));
 
 	return 0;
@@ -556,6 +554,12 @@
 
 	down(&(scn->sem));
 
+	if (!scn->present) {
+		/* The device was unplugged while open */
+		up(&(scn->sem));
+		return -ENODEV;
+	}
+
 	if (!scn->bulk_out_ep) {
 		/* This scanner does not have a bulk-out endpoint */
 		up(&(scn->sem));
@@ -650,6 +654,12 @@
 
 	down(&(scn->sem));
 
+	if (!scn->present) {
+		/* The device was unplugged while open */
+		up(&(scn->sem));
+		return -ENODEV;
+	}
+
 	scn_minor = scn->scn_minor;
 
 	ibuf = scn->ibuf;
@@ -757,6 +767,12 @@
 	scn = file->private_data;
 	down(&(scn->sem));
 
+	if (!scn->present) {
+		/* The device was unplugged while open */
+		up(&(scn->sem));
+		return -ENODEV;
+	}
+
 	dev = scn->scn_dev;
 
 	switch (cmd)
@@ -1086,6 +1102,14 @@
 }
 
 static void
+purge_scanner(struct scn_usb_data *scn)
+{
+	kfree(scn->ibuf);
+	kfree(scn->obuf);
+	kfree(scn);
+}
+
+static void
 disconnect_scanner(struct usb_device *dev, void *ptr)
 {
 	struct scn_usb_data *scn = (struct scn_usb_data *) ptr;
@@ -1100,15 +1124,22 @@
         usb_driver_release_interface(&scanner_driver,
                 &scn->scn_dev->actconfig->interface[scn->ifnum]);
 
-	kfree(scn->ibuf);
-	kfree(scn->obuf);
-
 	dbg("disconnect_scanner: De-allocating minor:%d", scn->scn_minor);
 	devfs_unregister(scn->devfs);
 	p_scn_table[scn->scn_minor] = NULL;
+
+	if (scn->isopen) {
+		/* The device is still open - cleanup must be delayed */
+		scn->present = 0;
+		up(&(scn->sem));
+		up(&scn_mutex);
+		return;
+	}
+
 	up (&(scn->sem));
-	kfree (scn);
 	up (&scn_mutex);
+
+	purge_scanner(scn);
 }
 
 static struct