linux-kernel - Re: TTY: tty

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 for Android: free password hash cracker in your pocket

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

Date:	Sun, 25 Mar 2012 23:33:15 +0100
From:	Al Viro <viro@...IV.linux.org.uk>
To:	Alan Cox <alan@...rguk.ukuu.org.uk>
Cc:	Richard Weinberger <richard@....at>, Jiri Slaby <jslaby@...e.cz>,
	linux-kernel@...r.kernel.org, gregkh@...uxfoundation.org,
	Jiri Slaby <jirislaby@...il.com>
Subject: Re: TTY: tty_port questions

On Sun, Mar 25, 2012 at 10:06:10PM +0100, Alan Cox wrote:
> Serial had pre-exising gunge not all of which has been cleaned, just as
> it still has its own buffers that want to be using kfifo.
> 
> Best examples are probably USB serial and neatest may well be the sdio
> serial card driver.

Umm...  So we have
	* tty_port created for each physical device, index assigned to it
and tty_register_device() done to create sysfs junk/kick udev/etc.
	* TTY layer allocates tty on demand (open() time) and feeds them
to ->install(), which is where we associate the suckers with tty_port,
grabbing a reference to the latter and shoving it into ->driver_data (OK,
it or that to struct it's embedded into - all the same)
	* ->cleanup() is called when TTY layer decided to free tty; drops
a reference to port.  Nobody will see that tty_struct anymore.
	* port outlives tty; over the lifetime of the latter associated
port remains the same.
	* ->open()/->close()/->hungup() simply call tty_port_...()
[BTW, is there any reason why you do not set ->driver_data to port and
use container_of() in the places that want other parts of containing
struct?  That way you wouldn't need those wrappers at all and while
->write() is certainly called more often, there's no extra cost - compiler
is able to figure out that local variable remains equal to argument-constant
and turn accesses relative to it into ones relative to argument]
	* guts of opening the damn thing go into ->activate(), guts of
stopping - into ->shutdown().
	* removal does tty_unregister_device() + prevents ->install() from
finding it + (under port->mutex) does tty_hangup() on associated tty (if any).
BTW, I really don't like the look of that place - tty_hangup() is async
(otherwise it'd deadlock instantly), so what the devil is protecting tty
from being freed before __tty_hangup() is done with it?  And when should
the actual channel be killed?  It appears to be done right after tty_hangup()
returns, but since the actual work done by it is async... why is that safe?
	* ->activate() plays strange games with TTY_IO_ERROR; why do we
bother, seeing that it's under port->mutex and anybody trying to open the
same tty will wait anyway?
--
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/