From b68f2fb9e73f46037fbeca5fbd4ae8a7ddd8ef6b Mon Sep 17 00:00:00 2001 From: Alan Cox Date: Mon, 27 Jul 2009 10:58:08 +0100 Subject: tty: Fix a USB serial crash/scribble The port lock is used to protect the port state. However the port structure is freed on a hangup, then the lock taken on a close. The right fix is to drop the port on tty->shutdown() but we can't yet do that due to sleep v non-sleeping rules. Instead do the next best thing and fix it up when we are not in -rc season. Reported-by: Daniel Mack Signed-off-by: Alan Cox Tested-by: Daniel Mack Signed-off-by: Linus Torvalds --- drivers/usb/serial/usb-serial.c | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) (limited to 'drivers/usb') diff --git a/drivers/usb/serial/usb-serial.c b/drivers/usb/serial/usb-serial.c index bd7581b3a48..3c8923f62ed 100644 --- a/drivers/usb/serial/usb-serial.c +++ b/drivers/usb/serial/usb-serial.c @@ -340,6 +340,22 @@ static void serial_close(struct tty_struct *tty, struct file *filp) dbg("%s - port %d", __func__, port->number); + /* FIXME: + This leaves a very narrow race. Really we should do the + serial_do_free() on tty->shutdown(), but tty->shutdown can + be called from IRQ context and serial_do_free can sleep. + + The right fix is probably to make the tty free (which is rare) + and thus tty->shutdown() occur via a work queue and simplify all + the drivers that use it. + */ + if (tty_hung_up_p(filp)) { + /* serial_hangup already called serial_down at this point. + Another user may have already reopened the port but + serial_do_free is refcounted */ + serial_do_free(port); + return; + } if (tty_port_close_start(&port->port, tty, filp) == 0) return; @@ -355,7 +371,8 @@ static void serial_hangup(struct tty_struct *tty) struct usb_serial_port *port = tty->driver_data; serial_do_down(port); tty_port_hangup(&port->port); - serial_do_free(port); + /* We must not free port yet - the USB serial layer depends on it's + continued existence */ } static int serial_write(struct tty_struct *tty, const unsigned char *buf, -- cgit v1.2.3 From c56d300086140c93dc3390e5300fd17df802ec0e Mon Sep 17 00:00:00 2001 From: Alan Cox Date: Tue, 28 Jul 2009 00:34:58 +0100 Subject: usb_serial: Fix remaining ref count/lock bugs This fixes - locking bug that was hidden by ecc2e05e739c30870c8e4f252b63a0c4041f2724 - Regression #13821 - Spurious warning when closing and blocking for data write out With these changes my PL2303 always ends up as ttyUSB0 when it should and the module refcounts stay correct. I'll do a more wholesale split & tidy of _open in the next release or two as we get a standard tty_port_open and port->ops->init port->ops->shutdown call backs. Copy sent to Alan Stern and Carlos Mafra just to confirm it fixes all the reports but it passes local testing with the same hardware as Alan Stern. Signed-off-by: Alan Cox Signed-off-by: Linus Torvalds --- drivers/usb/serial/usb-serial.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) (limited to 'drivers/usb') diff --git a/drivers/usb/serial/usb-serial.c b/drivers/usb/serial/usb-serial.c index 3c8923f62ed..99188c92068 100644 --- a/drivers/usb/serial/usb-serial.c +++ b/drivers/usb/serial/usb-serial.c @@ -32,6 +32,7 @@ #include #include #include +#include #include #include #include "pl2303.h" @@ -184,6 +185,7 @@ static int serial_open (struct tty_struct *tty, struct file *filp) struct usb_serial_port *port; unsigned int portNumber; int retval = 0; + int first = 0; dbg("%s", __func__); @@ -223,7 +225,7 @@ static int serial_open (struct tty_struct *tty, struct file *filp) /* If the console is attached, the device is already open */ if (port->port.count == 1 && !port->console) { - + first = 1; /* lock this module before we call it * this may fail, which means we must bail out, * safe because we are called with BKL held */ @@ -246,13 +248,21 @@ static int serial_open (struct tty_struct *tty, struct file *filp) if (retval) goto bailout_interface_put; mutex_unlock(&serial->disc_mutex); + set_bit(ASYNCB_INITIALIZED, &port->port.flags); } mutex_unlock(&port->mutex); /* Now do the correct tty layer semantics */ retval = tty_port_block_til_ready(&port->port, tty, filp); - if (retval == 0) + if (retval == 0) { + if (!first) + usb_serial_put(serial); return 0; - + } + mutex_lock(&port->mutex); + if (first == 0) + goto bailout_mutex_unlock; + /* Undo the initial port actions */ + mutex_lock(&serial->disc_mutex); bailout_interface_put: usb_autopm_put_interface(serial->interface); bailout_module_put: @@ -411,7 +421,6 @@ static int serial_chars_in_buffer(struct tty_struct *tty) struct usb_serial_port *port = tty->driver_data; dbg("%s = port %d", __func__, port->number); - WARN_ON(!port->port.count); /* if the device was unplugged then any remaining characters fell out of the connector ;) */ if (port->serial->disconnected) -- cgit v1.2.3