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

Reply via email to