Hi,

this fixes:

1) serial table is not protected by locks. kref_get() may be called on freed 
memory
2) probe() exposes uninitialized devices
3) disconnect() does not make sure there's no more IO to unbound devices

Please test. It's against 2.6.20-rc3 but should apply widely.

        Regards
                Oliver
--- a/drivers/usb/serial/usb-serial.c	2007-01-01 01:53:20.000000000 +0100
+++ b/drivers/usb/serial/usb-serial.c	2007-01-02 10:44:08.000000000 +0100
@@ -59,14 +59,19 @@
 
 static int debug;
 static struct usb_serial *serial_table[SERIAL_TTY_MINORS];	/* initially all NULL */
+static spinlock_t table_lock;
 static LIST_HEAD(usb_serial_driver_list);
 
 struct usb_serial *usb_serial_get_by_index(unsigned index)
 {
-	struct usb_serial *serial = serial_table[index];
+	struct usb_serial *serial;
+
+	spin_lock(&table_lock);
+	serial = serial_table[index];
 
 	if (serial)
 		kref_get(&serial->kref);
+	spin_unlock(&table_lock);
 	return serial;
 }
 
@@ -78,6 +83,7 @@
 	dbg("%s %d", __FUNCTION__, num_ports);
 
 	*minor = 0;
+	spin_lock(&table_lock);
 	for (i = 0; i < SERIAL_TTY_MINORS; ++i) {
 		if (serial_table[i])
 			continue;
@@ -96,8 +102,10 @@
 		dbg("%s - minor base = %d", __FUNCTION__, *minor);
 		for (i = *minor; (i < (*minor + num_ports)) && (i < SERIAL_TTY_MINORS); ++i)
 			serial_table[i] = serial;
+		spin_unlock(&table_lock);
 		return serial;
 	}
+	spin_unlock(&table_lock);
 	return NULL;
 }
 
@@ -110,9 +118,11 @@
 	if (serial == NULL)
 		return;
 
+	spin_lock(&serial_table);
 	for (i = 0; i < serial->num_ports; ++i) {
 		serial_table[serial->minor + i] = NULL;
 	}
+	spin_unlock(&serial_table);
 }
 
 static void destroy_serial(struct kref *kref)
@@ -559,15 +569,20 @@
 	port_free(port);
 }
 
-static void port_free(struct usb_serial_port *port)
+static void kill_traffic(struct usb_serial_port *port)
 {
 	usb_kill_urb(port->read_urb);
-	usb_free_urb(port->read_urb);
 	usb_kill_urb(port->write_urb);
-	usb_free_urb(port->write_urb);
 	usb_kill_urb(port->interrupt_in_urb);
-	usb_free_urb(port->interrupt_in_urb);
 	usb_kill_urb(port->interrupt_out_urb);
+}
+
+static void port_free(struct usb_serial_port *port)
+{
+	kill_traffic(port);
+	usb_free_urb(port->read_urb);
+	usb_free_urb(port->write_urb);
+	usb_free_urb(port->interrupt_in_urb);
 	usb_free_urb(port->interrupt_out_urb);
 	kfree(port->bulk_in_buffer);
 	kfree(port->bulk_out_buffer);
@@ -771,12 +786,6 @@
 			num_ports = type->num_ports;
 	}
 
-	if (get_free_serial (serial, num_ports, &minor) == NULL) {
-		dev_err(&interface->dev, "No more free serial devices\n");
-		kfree (serial);
-		return -ENOMEM;
-	}
-
 	serial->minor = minor;
 	serial->num_ports = num_ports;
 	serial->num_bulk_in = num_bulk_in;
@@ -925,6 +934,11 @@
 		}
 	}
 
+	if (get_free_serial (serial, num_ports, &minor) == NULL) {
+		dev_err(&interface->dev, "No more free serial devices\n");
+		goto probe_error;
+	}
+
 	/* register all of the individual ports with the driver core */
 	for (i = 0; i < num_ports; ++i) {
 		port = serial->port[i];
@@ -1002,8 +1016,11 @@
 	if (serial) {
 		for (i = 0; i < serial->num_ports; ++i) {
 			port = serial->port[i];
-			if (port && port->tty)
-				tty_hangup(port->tty);
+			if (port) {
+				if (port->tty)
+					tty_hangup(port->tty);
+				kill_traffic(port);
+			}
 		}
 		/* let the last holder of this object 
 		 * cause it to be cleaned up */
@@ -1040,6 +1057,7 @@
 		return -ENOMEM;
 
 	/* Initialize our global data */
+	spin_lock_init(&table_lock);
 	for (i = 0; i < SERIAL_TTY_MINORS; ++i) {
 		serial_table[i] = NULL;
 	}
-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel

Reply via email to