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
_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
