Hi, No, I don't see how it can work. When a device is disconnected serial_open will fail at the very beginning (usb_serial_get_by_index returns NULL in my patch or lock_serial fails in your earlier patch). In both cases bailout_module_put is bypassed and ++port->cleaning_up; is not executed.
But there may be a more serious problem. port is stored in tty->driver_data and is shared among all files on the same device. So, I wonder whether the following may happen: proc A -> serial_open file 1 - success proc B -> serial_open file 2 - failure - set cleaning_up flag proc A -> serial_close file 1 - aborted because of cleaning_up flag proc B -> serial_close file 2 - proceeded to close file 2 even if it was never opened. The only way to differentiate between file 1 and 2 is by filp, but currently filp is not used by usb-serial.c - it is simply passed to serial->type->open(port, filp); and port->serial->type->close(port, filp); and who knows how it may be used there? By the way, did you receive my patch, which I sent on Friday for review? E-Mail title was called "[PATCH 1/1] race condition fixes for usb-serial - new attempt - for review" Thank you John --- Oliver Neukum <[EMAIL PROTECTED]> wrote: > Hi, > > it turns out that tty_open() calls close() even if > open() has failed. > As this is undesireable for usb serial drivers, > here's a patch > to deal with it. > > John, does it fix the issue you reported? > > Regards > Oliver > -- > > --- a/include/linux/usb/serial.h 2007-03-26 > 09:23:20.000000000 +0200 > +++ b/include/linux/usb/serial.h 2007-03-26 > 09:27:12.000000000 +0200 > @@ -92,6 +92,7 @@ > int open_count; > char throttled; > char throttle_req; > + int cleaning_up; > struct device dev; > }; > #define to_usb_serial_port(d) container_of(d, > struct usb_serial_port, dev) > --- a/drivers/usb/serial/usb-serial.c 2007-03-26 > 09:18:08.000000000 +0200 > +++ b/drivers/usb/serial/usb-serial.c 2007-03-26 > 09:28:14.000000000 +0200 > @@ -235,6 +235,7 @@ > return 0; > > bailout_module_put: > + ++port->cleaning_up; > module_put(serial->type->driver.owner); > bailout_mutex_unlock: > port->open_count = 0; > @@ -257,6 +258,12 @@ > > mutex_lock(&port->mutex); > > + if (port->cleaning_up) { > + --port->cleaning_up; > + mutex_unlock(&port->mutex); > + return; > + } > + > if (port->open_count == 0) { > mutex_unlock(&port->mutex); > return; > > ____________________________________________________________________________________ Never miss an email again! Yahoo! Toolbar alerts you the instant new Mail arrives. http://tools.search.yahoo.com/toolbar/features/mail/ ------------------------------------------------------------------------- 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