linux-kernel - Re: [GIT PATCH] TTY patches for 2.6.36

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: <201008092038.49439.arnd@arndb.de>
Date:	Mon, 9 Aug 2010 20:38:49 +0200
From:	Arnd Bergmann <arnd@...db.de>
To:	Andy Whitcroft <apw@...onical.com>
Cc:	Greg KH <gregkh@...e.de>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Alan Cox <alan@...rguk.ukuu.org.uk>,
	Andrew Morton <akpm@...ux-foundation.org>,
	linux-kernel@...r.kernel.org
Subject: Re: [GIT PATCH] TTY patches for 2.6.36

On Monday 09 August 2010, Andy Whitcroft wrote:
> With the new locking the BTM is held in place of the BKL, which
> effectivly means throughout open and close processing, which
> effectively means all TTY open/closes are serialised throughout
> regardless of the length of the shutdown processing.  The EIO appears
> to be no longer returnable.
> 
> Is this change in semantics expected?  If not, it is likely something
> which could be addressed separately after merging, as long as we are
> aware.

I audited (or tried to) all code that is able to sleep while holding the
BTM. In the cases that looked like they might sleep long, I added explicit
tty_unlock()/tty_lock() pairs around the sleeping code.

Code that potentially sleeps but should not do so for an indefinite
time (e.g. kmalloc(GFP_KERNEL)), I did not do this and still hold the BKL.

This means the code shifted to holding the BTM more often than it used
to hold the BKL, but for cases where we wait on user or hardware interaction,
it should still behave as it used to. If you found a place where this
is not true, that is probably an oversight and we should add explicit
tty_unlock() statements around it.

The problem is that we cannot easily give up the BTM while already holding
tty_port->mutex, tty_port->buf_mutex, console_semaphore, tty->termios_mutex
or tty->atomic_write_lock, which would give an AB-BA deadlock without
the autorelease semantics of the BKL. Similarly, any wait_event or
flush_work_queue potentially has the same problem (besides the fact that
it can self-deadlock what it waits for tried to take the BTM). Auditing
all these was the bulk of the work for the conversion.

I did not specifically audit for the case where tty_open() returns -EIO,
but I'd hope that it would be covered by the method I used for the
conversion.

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