netdev - Re: [PATCH] ircomm: release tty before sleeping potentially indefintely

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: <1362359178.3221.118.camel@thor.lan>
Date:	Sun, 03 Mar 2013 20:06:18 -0500
From:	Peter Hurley <peter@...leysoftware.com>
To:	David Miller <davem@...emloft.net>
Cc:	sasha.levin@...cle.com, samuel@...tiz.org,
	gregkh@...uxfoundation.org, jslaby@...e.cz, netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] ircomm: release tty before sleeping potentially
 indefintely

On Sun, 2013-03-03 at 19:33 -0500, David Miller wrote:
> From: Peter Hurley <peter@...leysoftware.com>
> Date: Sun, 03 Mar 2013 19:04:25 -0500
> 
> > All these are re-tested in the loop. What state test isn't repeated?
> 
> One that rechecks the non-blocking filp flag, the
> TTY_IO_ERROR tty flag and the termios settings.
> 
> Like I said, all of the state tests performed at the beginning of
> this function, before enterring the loop.

How is O_NONBLOCK going to change? This function is sitting on the
user-space open.

The filp parameter is only on this task stack. It hasn't been linked in
anywhere else. Because of course the file isn't open yet because this
function hasn't returned success.

The TTY_IO_ERROR flag is used by drivers (this one included) to turn
away concurrent reads and writes when shutting down. The tty core does
not set this. Now this driver might set this, if commanded to hangup via
ircomm_tty_hangup, but like I said that's already handled in the loop
by testing tty_hung_up_p.

The initial termios setting cflag settings are set by the driver open().
In this driver, its here:

	driver->init_termios    = tty_std_termios;
	driver->init_termios.c_cflag = B9600 | CS8 | CREAD | HUPCL | CLOCAL;

Now, it's possible that one could construct an imaginary race, where the
tty has already been opened and that task now sets the termios without
CLOCAL and meanwhile a second task is racing this termios setting with
an open() of its own, but since there is no expectation from userspace
that those operations are serialized, there's no reason to serialize
them here.

But regardless, this function __cannot__ sleep holding the tty_lock().

Regards,
Peter Hurley

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html