From: Alan Cox <alan@lxorguk.ukuu.org.uk>
To: daniel@caiaq.de, linux-kernel@vger.kernel.org,
linux-usb@vger.kernel.org, stern@rowland.harvard.edu
Subject: [PATCH] tty: Fix a USB serial crash/scribble
Date: Wed, 22 Jul 2009 10:39:51 +0100 [thread overview]
Message-ID: <20090722093735.27118.36158.stgit@localhost.localdomain> (raw)
See if this one looks sensible. It does leave a tiny race window but that
semes wiser than hacking up the tty kref_put path in the middle of an -rc
series.
Thanks to Daniel and Alan Stern for chasing this down and getting traces. Also
to Daniel for being persistent when I took it as a random odd "only seen by one
user" error which it wasn't.
---
From: Alan Cox <alan@linux.intel.com>
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 <daniel@caiaq.de>
Signed-off-by: Alan Cox <alan@linux.intel.com>
---
drivers/usb/serial/usb-serial.c | 19 ++++++++++++++++++-
1 files changed, 18 insertions(+), 1 deletions(-)
diff --git a/drivers/usb/serial/usb-serial.c b/drivers/usb/serial/usb-serial.c
index bd7581b..228d77c 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,
next reply other threads:[~2009-07-22 9:41 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-07-22 9:39 Alan Cox [this message]
2009-07-22 10:16 ` [PATCH] tty: Fix a USB serial crash/scribble Daniel Mack
2009-07-25 4:48 ` Greg KH
2009-07-25 11:56 ` Alan Cox
2009-07-25 16:55 ` Greg KH
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20090722093735.27118.36158.stgit@localhost.localdomain \
--to=alan@lxorguk.ukuu.org.uk \
--cc=daniel@caiaq.de \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=stern@rowland.harvard.edu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.