linux-kernel - Re: [Bug #14388] keyboard under X with 2.6.31

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]

Date:	Tue, 13 Oct 2009 15:42:56 -0700 (PDT)
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Paul Fulghum <paulkf@...rogate.com>
cc:	Boyan <btanastasov@...oo.co.uk>,
	"Frédéric L. W. Meunier" 
	<fredlwm@...il.com>, "Justin P. Mattock" <justinmattock@...il.com>,
	Nix <nix@...eri.org.uk>, Alan Cox <alan@...rguk.ukuu.org.uk>,
	"Rafael J. Wysocki" <rjw@...k.pl>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Kernel Testers List <kernel-testers@...r.kernel.org>,
	Dmitry Torokhov <dmitry.torokhov@...il.com>,
	Ed Tomlinson <edt@....ca>,
	OGAWA Hirofumi <hirofumi@...l.parknet.co.jp>
Subject: Re: [Bug #14388] keyboard under X with 2.6.31

On Tue, 13 Oct 2009, Paul Fulghum wrote:
> 
> If flush_to_ldisc() is reentered with the head set to null, nothing
> is done. New buffers can be added where you say, but they are
> added to the tail. So the order of flushed data is retained.

They are added to the tail only if the tail is non-NULL.

And buf.tail, in turn, is protected by the TTY_FLUSHING bit.

And look what happens to TTY_FLUSHING if flush_to_ldisc() is called by 
multiple contexts - it doesn't nest right. The inner "flush_to_ldisc()" 
will clear the bit (your "nothing is done" case).

Now, I agree that we can solve things differently. We could, for example, 
get rid of TTY_FLUSHING entirely. If you want to keep the crazy "head = 
NULL" special case, we could basically replace all tests of TTY_FLUSHING 
with "tty->buf.tail && !tty->buf.head" instead, and use _that_ as a "the 
TTY is in the middle of a flush" operation. That should be 100% equivalent 
to my patch.

I do object to the whole crazy subtle TTY locking. I'm convinced it's 
wrong, and I'm convinced it's wrong exactly _because_ it tries to be so 
subtle and does non-obvious things.

That's why my patch also changed the whole loop logic: it's not subtle any 
more. Not only did I make TTY_FLUSHING nest correctly, I also stopped 
playing games with buf.head: it's now purely a list, rather than "a list 
and a failed attempt to lock".

And no, I'm not sure my patch helps. I'd have expected 
'tty_buffer_flush()' to be something very rare, for example. But I also 
didn't really check if we may do it some other way.

But I _am_ sure that it makes the code a whole lot more straightforward. 
Bits that say "we're busy flushing" suddenly actually act that way, and 
pointers that say "this is the head of the buffers" also act that wy.

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