linux-kernel - Re: [PATCH tty-next 14/22] tty: Remove tty_wait_until_sent_from

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: <5434B5D3.4060308@hurleysoftware.com>
Date:	Tue, 07 Oct 2014 23:56:03 -0400
From:	Peter Hurley <peter@...leysoftware.com>
To:	David Laight <David.Laight@...LAB.COM>,
	Arnd Bergmann <arnd@...db.de>,
	"linuxppc-dev@...ts.ozlabs.org" <linuxppc-dev@...ts.ozlabs.org>,
	One Thousand Gnomes <gnomes@...rguk.ukuu.org.uk>
CC:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Karsten Keil <isdn@...ux-pingi.de>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-serial@...r.kernel.org" <linux-serial@...r.kernel.org>
Subject: Re: [PATCH tty-next 14/22] tty: Remove tty_wait_until_sent_from_close()

On 06/17/2014 07:03 AM, David Laight wrote:
> From: Peter Hurley
> ...
>>> I don't understand the second half of the changelog, it doesn't seem
>>> to fit here: there deadlock that we are trying to avoid here happens
>>> when the *same* tty needs the lock to complete the function that
>>> sends the pending data. I don't think we do still do that any more,
>>> but it doesn't seem related to the tty lock being system-wide or not.
>>
>> The tty lock is not used in the i/o path; it's purpose is to
>> mutually exclude state changes in open(), close() and hangup().
>>
>> The commit that added this [1] comments that _other_ ttys may wait
>> for this tty to complete, and comments in the code note that this
>> function should be removed when the system-wide tty mutex was removed
>> (which happened with the commit noted in the changelog).

I just wanted to revisit this discussion briefly so I can clarify the
situation regarding holding the tty lock while closing, and how that
affects parallel opens.

I've unnested the tty lock from the tty mutex (which I'm still testing)
but will be submitting after the merge window re-opens for 3.19. So this
is more relevant now.

The original patch that led to this thread is here:
https://lkml.org/lkml/2014/6/16/306

> What happens if another process tries to do a non-blocking open
> while you are sleeping in close waiting for output to drain?
> 
> Hopefully this returns before that data has drained.

Current mainline blocks on _any_ racing re-open while this lock is
dropped in tty_wait_until_sent_from_close(); blocking while
ASYNC_CLOSING has been in mainline since at least 2.6.29 and that just
merged existing code together. See tty_port_block_til_ready(); note
the test for O_NONBLOCK is after the wait while ASYNC_CLOSING.

IOW, currently a non-blocking open will sleep for the _entire_ duration
of a parallel hardware shutdown, and when it wakes, the error return will
cause a release of its tty, and it will restart with a fresh attempt
to open. Same with a blocking open that is already waiting; when its
woken the hardware shutdown has already completed so ASYNC_INITIALIZED
is cleared, which forces a release and restart too.

The point being that holding the tty lock across the _entire_ close
is equivalent to the current outcome, regardless of O_NONBLOCK.

I'm reluctant to start returning EGAIN for non-blocking tty opens
because no tty driver does that now, and I don't think userspace will
deal well with new return codes from tty opens.

Regards,
Peter Hurley

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