Browse Source

USB: cdc-acm: Fix disconnect() vs close() race

There's a race between the USB disconnect handler and the TTY close
handler which may cause the acm object to be freed while it's still
being used. This may lead to things like

http://article.gmane.org/gmane.linux.usb.general/54250

and

https://lkml.org/lkml/2011/5/29/64

This is the simplest fix I could come up with. Holding on to open_mutex
while closing the TTY device prevents acm_disconnect() from freeing the
acm object between acm->port.count drops to 0 and the TTY side of the
cleanups are finalized.

Signed-off-by: Havard Skinnemoen <hskinnemoen@google.com>
Cc: Oliver Neukum <oliver@neukum.name>
Cc: stable <stable@vger.kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
Havard Skinnemoen 13 years ago
parent
commit
5dc2470c60
1 changed files with 5 additions and 3 deletions
  1. 5 3
      drivers/usb/class/cdc-acm.c

+ 5 - 3
drivers/usb/class/cdc-acm.c

@@ -539,7 +539,6 @@ static void acm_port_down(struct acm *acm)
 {
 {
 	int i;
 	int i;
 
 
-	mutex_lock(&open_mutex);
 	if (acm->dev) {
 	if (acm->dev) {
 		usb_autopm_get_interface(acm->control);
 		usb_autopm_get_interface(acm->control);
 		acm_set_control(acm, acm->ctrlout = 0);
 		acm_set_control(acm, acm->ctrlout = 0);
@@ -551,14 +550,15 @@ static void acm_port_down(struct acm *acm)
 		acm->control->needs_remote_wakeup = 0;
 		acm->control->needs_remote_wakeup = 0;
 		usb_autopm_put_interface(acm->control);
 		usb_autopm_put_interface(acm->control);
 	}
 	}
-	mutex_unlock(&open_mutex);
 }
 }
 
 
 static void acm_tty_hangup(struct tty_struct *tty)
 static void acm_tty_hangup(struct tty_struct *tty)
 {
 {
 	struct acm *acm = tty->driver_data;
 	struct acm *acm = tty->driver_data;
 	tty_port_hangup(&acm->port);
 	tty_port_hangup(&acm->port);
+	mutex_lock(&open_mutex);
 	acm_port_down(acm);
 	acm_port_down(acm);
+	mutex_unlock(&open_mutex);
 }
 }
 
 
 static void acm_tty_close(struct tty_struct *tty, struct file *filp)
 static void acm_tty_close(struct tty_struct *tty, struct file *filp)
@@ -569,8 +569,9 @@ static void acm_tty_close(struct tty_struct *tty, struct file *filp)
 	   shutdown */
 	   shutdown */
 	if (!acm)
 	if (!acm)
 		return;
 		return;
+
+	mutex_lock(&open_mutex);
 	if (tty_port_close_start(&acm->port, tty, filp) == 0) {
 	if (tty_port_close_start(&acm->port, tty, filp) == 0) {
-		mutex_lock(&open_mutex);
 		if (!acm->dev) {
 		if (!acm->dev) {
 			tty_port_tty_set(&acm->port, NULL);
 			tty_port_tty_set(&acm->port, NULL);
 			acm_tty_unregister(acm);
 			acm_tty_unregister(acm);
@@ -582,6 +583,7 @@ static void acm_tty_close(struct tty_struct *tty, struct file *filp)
 	acm_port_down(acm);
 	acm_port_down(acm);
 	tty_port_close_end(&acm->port, tty);
 	tty_port_close_end(&acm->port, tty);
 	tty_port_tty_set(&acm->port, NULL);
 	tty_port_tty_set(&acm->port, NULL);
+	mutex_unlock(&open_mutex);
 }
 }
 
 
 static int acm_tty_write(struct tty_struct *tty,
 static int acm_tty_write(struct tty_struct *tty,