linux-kernel - Re: [PATCH] [usb-serial] fix Ooops on uplug

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20090722171707.5fa39803@lxorguk.ukuu.org.uk>
Date:	Wed, 22 Jul 2009 17:17:07 +0100
From:	Alan Cox <alan@...rguk.ukuu.org.uk>
To:	Alan Stern <stern@...land.harvard.edu>
Cc:	Daniel Mack <daniel@...aq.de>,
	Kernel development list <linux-kernel@...r.kernel.org>,
	USB list <linux-usb@...r.kernel.org>
Subject: Re: [PATCH] [usb-serial] fix Ooops on uplug

> > 		free resources
> > 		wake any pending openers
> 
> Where exactly is the code that wakes the other openers?

tty_port_close_end wakes port->open_wait

if another opener was blocked during the hangup they then exit
tty_blocked_until_ready and error

if another open occurs after the hangup completes (but while the other
use has the port open bug hung up then they will execute

	serial_open()
		->type->open()

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

For the case where it needs to do that the hangup will have finished and
it will get another call to serial_open()

> 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 core hangup and close code are interlocked (now - didn't use to be).

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

tty_hung_up_p() will be true

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

Beats me - not something I designed. However the driver would always need
to be aware of it because the following can occur

			CPU1			CPU2
			close begins
						hangup
						update ops
			close handler runs

The tty_port code handles that internally, but has to handle it anyway.

There are similar issues with all the other calls if they are pending and
I've not even begun to tackle them yet as they are basically
inconveniences only. Also because I'm still hoping someone will implement
revoke() on Linux and do all the hard work for me.

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

The termios flags control this behaviour.

Alan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/