Am Dienstag, 20. März 2007 00:56 schrieb John:
> This is a fix for race condition in usb-serial.c
> between serial_open and usb_serial_disconnect
> (kernel version 2.6.21-rc3)
> Prior to this patch instances of usb_serial were not
> ref-counted consistently, which caused serial_open
> to reuse already deleted instance.
> For more details search Web for
> "Possible race condition in usb-serial.c"
> discussion in Dec 2006.
Hi John,
this was so long ago that I made a patch based on the discussion
we had back then. Would you mind having a look at it? We should
merge those patches to get an optimum.
Regards
Oliver
--- a/include/linux/usb/serial.h 2007-03-16 01:32:25.000000000 +0100
+++ b/include/linux/usb/serial.h 2007-03-16 20:32:05.000000000 +0100
@@ -128,6 +128,7 @@
struct usb_device * dev;
struct usb_serial_driver * type;
struct usb_interface * interface;
+ spinlock_t lock;
unsigned char minor;
unsigned char num_ports;
unsigned char num_port_pointers;
@@ -135,6 +136,7 @@
char num_interrupt_out;
char num_bulk_in;
char num_bulk_out;
+ int busy;
struct usb_serial_port * port[MAX_NUM_PORTS];
struct kref kref;
void * private;
--- a/drivers/usb/serial/usb-serial.c 2007-03-16 01:32:38.000000000 +0100
+++ b/drivers/usb/serial/usb-serial.c 2007-03-16 21:24:47.000000000 +0100
@@ -59,8 +59,10 @@
static int debug;
static struct usb_serial *serial_table[SERIAL_TTY_MINORS]; /* initially
all NULL */
+static unsigned long deathrow[SERIAL_TTY_MINORS/BITS_PER_LONG];
/* disconnected but open*/
static spinlock_t table_lock;
static LIST_HEAD(usb_serial_driver_list);
+static DECLARE_WAIT_QUEUE_HEAD(usbserial_henchman);
struct usb_serial *usb_serial_get_by_index(unsigned index)
{
@@ -104,6 +106,7 @@
for (i = *minor; (i < (*minor + num_ports)) && (i <
SERIAL_TTY_MINORS); ++i) {
serial_table[i] = serial;
serial->port[j++]->number = i;
+ __clear_bit(i, deathrow);
}
spin_unlock(&table_lock);
return serial;
@@ -112,6 +115,53 @@
return NULL;
}
+/* requires locking from caller*/
+static int on_deathrow(struct usb_serial_port *port)
+{
+ int number = port->number;
+
+ return test_bit(number, deathrow);
+}
+
+/* requires locking from caller*/
+static void schedule_execution(struct usb_serial_port *port)
+{
+ int number = port->number;
+
+ __set_bit(number, deathrow);
+}
+
+static int lock_serial(struct usb_serial_port *port)
+{
+ struct usb_serial *serial = port->serial;
+ unsigned long flags;
+ int terminated;
+
+ spin_lock_irqsave(&serial->lock, flags);
+ terminated = on_deathrow(port);
+ if (!terminated)
+ serial->busy++;
+ spin_unlock_irqrestore(&serial->lock, flags);
+
+ if (terminated)
+ return -ENODEV;
+ return 0;
+}
+
+static void unlock_serial(struct usb_serial_port *port)
+{
+ struct usb_serial *serial = port->serial;
+ unsigned long flags;
+ int waiters;
+
+ spin_lock_irqsave(&serial->lock, flags);
+ waiters = --serial->busy;
+ spin_unlock_irqrestore(&serial->lock, flags);
+
+ if (!waiters)
+ wake_up(&usbserial_henchman);
+}
+
static void return_serial(struct usb_serial *serial)
{
int i;
@@ -148,9 +198,6 @@
serial->port[i] = NULL;
}
- if (serial->type->shutdown)
- serial->type->shutdown(serial);
-
/* return the minor range that this device had */
return_serial(serial);
@@ -203,9 +250,13 @@
goto bailout_kref_put;
}
+ retval = lock_serial(port);
+ if (retval)
+ goto bailout_kref_put;
+
if (mutex_lock_interruptible(&port->mutex)) {
retval = -ERESTARTSYS;
- goto bailout_kref_put;
+ goto bailout_kref_put_unlock;
}
++port->open_count;
@@ -233,6 +284,7 @@
}
mutex_unlock(&port->mutex);
+ unlock_serial(port);
return 0;
bailout_module_put:
@@ -242,6 +294,8 @@
tty->driver_data = NULL;
port->tty = NULL;
mutex_unlock(&port->mutex);
+bailout_kref_put_unlock:
+ unlock_serial(port);
bailout_kref_put:
usb_serial_put(serial);
return retval;
@@ -298,9 +352,15 @@
goto exit;
}
+ retval = lock_serial(port);
+ if (retval)
+ goto exit;
+
/* pass on to the driver specific version of this function */
retval = port->serial->type->write(port, buf, count);
+ unlock_serial(port);
+
exit:
return retval;
}
@@ -320,9 +380,14 @@
goto exit;
}
+ retval = lock_serial(port);
+ if (retval)
+ goto exit;
/* pass on to the driver specific version of this function */
retval = port->serial->type->write_room(port);
+ unlock_serial(port);
+
exit:
return retval;
}
@@ -342,9 +407,13 @@
goto exit;
}
+ retval = lock_serial(port);
+ if (retval)
+ goto exit;
/* pass on to the driver specific version of this function */
retval = port->serial->type->chars_in_buffer(port);
+ unlock_serial(port);
exit:
return retval;
}
@@ -364,8 +433,12 @@
}
/* pass on to the driver specific version of this function */
- if (port->serial->type->throttle)
+ if (port->serial->type->throttle) {
+ if (0 < lock_serial(port))
+ return;
port->serial->type->throttle(port);
+ unlock_serial(port);
+ }
}
static void serial_unthrottle (struct tty_struct * tty)
@@ -383,8 +456,12 @@
}
/* pass on to the driver specific version of this function */
- if (port->serial->type->unthrottle)
+ if (port->serial->type->unthrottle) {
+ if (0 < lock_serial(port))
+ return;
port->serial->type->unthrottle(port);
+ unlock_serial(port);
+ }
}
static int serial_ioctl (struct tty_struct *tty, struct file * file, unsigned
int cmd, unsigned long arg)
@@ -402,12 +479,16 @@
goto exit;
}
+ retval = lock_serial(port);
+ if (retval)
+ goto exit;
/* pass on to the driver specific version of this function if it is
available */
if (port->serial->type->ioctl)
retval = port->serial->type->ioctl(port, file, cmd, arg);
else
retval = -ENOIOCTLCMD;
-
+
+ unlock_serial(port);
exit:
return retval;
}
@@ -427,8 +508,12 @@
}
/* pass on to the driver specific version of this function if it is
available */
- if (port->serial->type->set_termios)
+ if (port->serial->type->set_termios) {
+ if (0 < lock_serial(port))
+ return;
port->serial->type->set_termios(port, old);
+ unlock_serial(port);
+ }
}
static void serial_break (struct tty_struct *tty, int break_state)
@@ -446,8 +531,12 @@
}
/* pass on to the driver specific version of this function if it is
available */
- if (port->serial->type->break_ctl)
+ if (port->serial->type->break_ctl) {
+ if (0 < lock_serial(port))
+ return;
port->serial->type->break_ctl(port, break_state);
+ unlock_serial(port);
+ }
}
static int serial_read_proc (char *page, char **start, off_t off, int count,
int *eof, void *data)
@@ -500,6 +589,7 @@
static int serial_tiocmget (struct tty_struct *tty, struct file *file)
{
struct usb_serial_port *port = tty->driver_data;
+ int retval;
if (!port)
return -ENODEV;
@@ -511,16 +601,26 @@
return -ENODEV;
}
+ retval = lock_serial(port);
+ if (retval)
+ goto exit;
+
if (port->serial->type->tiocmget)
- return port->serial->type->tiocmget(port, file);
+ retval = port->serial->type->tiocmget(port, file);
+ else
+ retval = -EINVAL;
+
+ unlock_serial(port);
+exit:
- return -EINVAL;
+ return retval;
}
static int serial_tiocmset (struct tty_struct *tty, struct file *file,
unsigned int set, unsigned int clear)
{
struct usb_serial_port *port = tty->driver_data;
+ int retval;
if (!port)
return -ENODEV;
@@ -532,10 +632,18 @@
return -ENODEV;
}
+ retval = lock_serial(port);
+ if (retval)
+ goto exit;
+
if (port->serial->type->tiocmset)
- return port->serial->type->tiocmset(port, file, set, clear);
+ retval = port->serial->type->tiocmset(port, file, set, clear);
+ else
+ retval = -EINVAL;
- return -EINVAL;
+ unlock_serial(port);
+exit:
+ return retval;
}
/*
@@ -612,6 +720,7 @@
serial->type = driver;
serial->interface = interface;
kref_init(&serial->kref);
+ spin_lock_init(&serial->lock);
return serial;
}
@@ -1059,11 +1168,19 @@
for (i = 0; i < serial->num_ports; ++i) {
port = serial->port[i];
if (port) {
+ spin_lock_irq(&serial->lock);
+ schedule_execution(port);
+ spin_unlock_irq(&serial->lock);
if (port->tty)
tty_hangup(port->tty);
kill_traffic(port);
}
}
+ wait_event(usbserial_henchman, serial->busy == 0);
+ /* allow the subdriver to cleanly kill
+ * private URBs */
+ if (serial->type->shutdown)
+ (serial->type->shutdown)(serial);
/* let the last holder of this object
* cause it to be cleaned up */
usb_serial_put(serial);
-------------------------------------------------------------------------
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
_______________________________________________
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
