lkml.org 
[lkml]   [2009]   [Jul]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] [usb-serial] fix Ooops on uplug
On Tue, 21 Jul 2009, Alan Cox wrote:

> The sequences of behaviour for the tty interface are usually
>
> open
> allocate resources
> start uart
>
> do stuff
>
> close
> stop uart
> free resources
>
> and if a hang up occurs:
>
> open
> allocate resources
> start uart
> do stuff
>
> hangup event
> stop uart
> free resources
> wake any pending openers

Where exactly is the code that wakes the other openers?

>
> [pending open completes]

Does this completion involve calling tty->ops->open()? Or was it
called earlier, before the open was forced to block?

> allocate resources
> start uart

To rephrase the question above, how is the device driver made aware
that it needs to reallocate resources and restart the uart?

> [original opener closes]
> close
> was hung up
> do nothing much

There's an obvious race here between hangup and close. The assignment
of hung_up_tty_fops to filp->f_op is protected by the BKL and not much
else. Does the code in tty_release_dev() check to see whether this
assignment has been made before calling tty->ops->close()? It doesn't
like like it to me. With the wrong timing, you could end up telling
the device driver to stop the uart twice.


> The tty notion of "open" is really open->hangup or open->close. Once the
> hangup occurs you may have a file handle to a tty object but it doesn't
> talk to hardware.

But it still talks to the device driver via tty_release_dev's call to
tty->ops->close. How is the driver supposed to know that this method
call shouldn't talk to the hardware?

In fact, what point is there in making the call at all? Once the
hangup has occurred, the driver shouldn't do _anything_ when the
corresponding release happens. As you say, the notion is open->hangup
or open->close, not open->(hangup followed by close).

> You need to open it again to get access again. Between
> hangup and close you are sitting on a dead file handle that returns
> errors if you use it. The hardware is owned by whoever opened it next.
>
> The historical model for this is from dial in. A carrier drop is
> effectively a secure attention key, it has to kill off anything left on
> the port so an evil user cannot leave a key logger active on the port.

What about the other historical model, a user sitting at a terminal and
telling his modem to dial-out? One wouldn't think the modem's device
file would need to be reopened between calls.

> > Eliminating the fake calls seems like the cleanest solution.
> > Alternatively, we could keep the fake close (but not the fake release!)
> > and change serial_close() so that it calls serial_do_free() even if
> > tty_port_close_start returns 0.
>
> There are several other cases where tty_port_close_start returns zero
> (such as multiple openers and not being the last one)

I've seen the patch you just posted, and I agree -- it isn't the right
fix for the long run. By all means, call tty->ops->shutdown in process
context and that's where we will release the port resources.

But the overall intent still isn't clear, not without the answers to
the questions above.

Alan Stern



\
 
 \ /
  Last update: 2009-07-22 16:47    [W:0.130 / U:0.476 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site