mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Alan Cox <alan@lxorguk.ukuu.org.uk>
To: Alan Stern <stern@rowland.harvard.edu>
Cc: Daniel Mack <daniel@caiaq.de>,
	Kernel development list <linux-kernel@vger.kernel.org>,
	USB list <linux-usb@vger.kernel.org>
Subject: Re: [PATCH] [usb-serial] fix Ooops on uplug
Date: Tue, 21 Jul 2009 23:55:12 +0100	[thread overview]
Message-ID: <20090721235512.7a1784be@lxorguk.ukuu.org.uk> (raw)
In-Reply-To: <Pine.LNX.4.44L0.0907211632080.2679-100000@iolanthe.rowland.org>

> > We can't just skip freeing the resources in the hangup method as
> > tty_port_close_start() will return 0 and leak them on a hangup.
> 
> Wait a second.  Does the same serial_hangup() routine get called when
> an RS-232 (for example) hangup event occurs, like DCD turning off, and
> when the USB device is unplugged?  Sure, the second implies the first,
> but they need to be treated differently.  After the first, the hardware
> is still present and so the port resources shouldn't be released.

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

	[pending open completes]
		allocate resources
		start uart

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


> Perhaps not; I'll explain.  It's very simplistic, because when I wrote 
> it I didn't know what was going on with the tty layer internals.  (I 
> still don't, come to that.)

Ok so between us we know the answer I hope 8)

> The usb_serial_port structures should exist as long as they are needed, 
> which means as long as the USB device is connected or the tty device 
> file is open.  That's how my original design was meant to work, but it 
> is now messed up by the fact that we get two "close" events -- a fake 
> one during disconnect and then the real one later.

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. 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.

> 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)

Alan

  reply	other threads:[~2009-07-21 22:54 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20090720155502.50b84ae9@lxorguk.ukuu.org.uk>
2009-07-20 17:51 ` Daniel Mack
2009-07-20 22:48   ` Alan Cox
2009-07-20 21:58     ` Daniel Mack
2009-07-20 23:45       ` Alan Cox
2009-07-21 15:53         ` Alan Stern
2009-07-21 15:56           ` Daniel Mack
2009-07-21 16:00             ` Alan Cox
2009-07-21 16:11               ` Alan Stern
2009-07-21 16:18                 ` Alan Cox
2009-07-21 16:16           ` Alan Cox
2009-07-21 16:19             ` Daniel Mack
2009-07-21 16:28               ` Alan Cox
2009-07-21 18:36             ` Alan Stern
2009-07-21 19:21               ` Alan Cox
2009-07-21 21:38                 ` Alan Stern
2009-07-21 22:55                   ` Alan Cox [this message]
2009-07-22 14:44                     ` Alan Stern
2009-07-22 16:17                       ` Alan Cox
2009-07-22 18:21                         ` Alan Stern
2009-07-22 22:35                           ` Alan Cox
2009-07-22 22:45                             ` Alan Stern
2009-07-22 22:48                               ` Alan Cox
2009-07-20 23:32   ` Alan Cox

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20090721235512.7a1784be@lxorguk.ukuu.org.uk \
    --to=alan@lxorguk.ukuu.org.uk \
    --cc=daniel@caiaq.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=stern@rowland.harvard.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®