[PATCH] USB: revised usbserial open/close unbalanced get/put

Here is a new version of the patch to usb-serial.c to fix
the unbalanced kref and module get/put in serial_open and
serial_close when there are errors.  This version works
correctly when try_module_get fails and now "rmmod --wait"
works correctly.


* If the low level usb serial open fails, the kref and
  module use counts are decremented once in open.  Before,
  they were mistakenly decremented again in close.

* If try_module_get fails, the module use count is not
  changed.  Before, the module use count was mistakenly
  decremented.  Now "rmmod --wait" works correctly.

* The tty->driver_data pointer is never set to NULL once
  the port is open.  Before there was a small window when
  it was mistakenly set to NULL for an already open port.
Signed-off-by: Al Borchers <alborchers@steinerpoint.com>
Signed-off-by: Greg Kroah-Hartman <greg@kroah.com>

drivers/usb/serial/usb-serial.c

@@ -480,45 +480,51 @@ static int serial_open (struct tty_struct *tty, struct file * filp)
 	struct usb_serial *serial;
 	struct usb_serial_port *port;
 	unsigned int portNumber;
-	int retval = -ENODEV;
+	int retval;
 	
 	dbg("%s", __FUNCTION__);
 
-	/* initialize the pointer incase something fails */
-	tty->driver_data = NULL;
-
 	/* get the serial object associated with this tty pointer */
 	serial = usb_serial_get_by_index(tty->index);
-	if (!serial)
-		goto bailout;
+	if (!serial) {
+		tty->driver_data = NULL;
+		return -ENODEV;
+	}
 
-	/* set up our port structure making the tty driver remember our port object, and us it */
 	portNumber = tty->index - serial->minor;
 	port = serial->port[portNumber];
-	tty->driver_data = port;
-
-	port->tty = tty;
 	 
-	/* lock this module before we call it,
-	   this may, which means we must bail out, safe because we are called with BKL held */
-	if (!try_module_get(serial->type->owner)) {
-		kref_put(&serial->kref, destroy_serial);
-		goto bailout;
-	}
-
-	retval = 0;
 	++port->open_count;
+
 	if (port->open_count == 1) {
+
+		/* set up our port structure making the tty driver
+		 * remember our port object, and us it */
+		tty->driver_data = port;
+		port->tty = tty;
+
+		/* lock this module before we call it
+		 * this may fail, which means we must bail out,
+		 * safe because we are called with BKL held */
+		if (!try_module_get(serial->type->owner)) {
+			retval = -ENODEV;
+			goto bailout_kref_put;
+		}
+
 		/* only call the device specific open if this 
 		 * is the first time the port is opened */
 		retval = serial->type->open(port, filp);
-		if (retval) {
-			port->open_count = 0;
-			module_put(serial->type->owner);
-			kref_put(&serial->kref, destroy_serial);
-		}
+		if (retval)
+			goto bailout_module_put;
 	}
-bailout:
+
+	return 0;
+
+bailout_module_put:
+	module_put(serial->type->owner);
+bailout_kref_put:
+	kref_put(&serial->kref, destroy_serial);
+	port->open_count = 0;
 	return retval;
 }
 
@@ -531,21 +537,24 @@ static void serial_close(struct tty_struct *tty, struct file * filp)
 
 	dbg("%s - port %d", __FUNCTION__, port->number);
 
+	if (port->open_count == 0)
+		return;
+
 	--port->open_count;
-	if (port->open_count <= 0) {
+	if (port->open_count == 0) {
 		/* only call the device specific close if this 
 		 * port is being closed by the last owner */
 		port->serial->type->close(port, filp);
-		port->open_count = 0;
 
 		if (port->tty) {
 			if (port->tty->driver_data)
 				port->tty->driver_data = NULL;
 			port->tty = NULL;
 		}
+
+		module_put(port->serial->type->owner);
 	}
 
-	module_put(port->serial->type->owner);
 	kref_put(&port->serial->kref, destroy_serial);
 }