linux-kernel - [PATCH 0/2] proper tty-ldisc refcounting (was Re: WARNING at: drivers/char/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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <alpine.LFD.2.01.0908031047010.3208@localhost.localdomain>
Date:	Mon, 3 Aug 2009 10:55:34 -0700 (PDT)
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Alan Cox <alan@...rguk.ukuu.org.uk>
cc:	OGAWA Hirofumi <hirofumi@...l.parknet.co.jp>,
	Sergey Senozhatsky <sergey.senozhatsky@...l.by>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Greg KH <greg@...ah.com>
Subject: [PATCH 0/2] proper tty-ldisc refcounting (was Re: WARNING at:
 drivers/char/tty_ldisc.c)

On Sun, 2 Aug 2009, Linus Torvalds wrote:
> 
> > You could just finish the ldisc refcounting. The last set of patches you
> > had off me split tty->ldisc from struct tty ready to do exactly that and
> > I don't think there is anything left that stops it happening now (It was
> > just not ready in time)
> 
> I considered it, and it didn't look horrible (the thing really is pretty 
> self-contained in tty_ldisc_try() and tty_ldisc_deref()).

Here we are.

It wasn't a straight conversion, because the old code really didn't think 
of the refcounts as lifetimes, but it wasn't too bad either. And doing the 
proper refcounting makes all the stupid "wait for idle" go away, so it 
actually removes code:

 drivers/char/tty_ldisc.c  |  145 ++++++++++++++------------------------------
 include/linux/tty_ldisc.h |    2 +-
 2 files changed, 47 insertions(+), 100 deletions(-)

and generally simplifies the logic.

That said, looking through the code as I did this, I consciously avoided 
doing some other cleanups that really should be done some day. The code is 
chock-full of crazy stuff, where we just do

	o_ldisc = tty->ldisc;

with dubious locking. None of that is _new_ though, and most of it is in 
the "replace one ldisc with another" code. And for all I know, maybe it's 
all fine, it's just very much not _obviously_ correct.

As far as I can tell, this short series should not introduce any new 
problems, but hey, maybe it leaks ldisc references like mad because I made 
some silly mistake. It's a _fairly_ straightforward cleanup, but it's a 
big cnnceptual change to go from a model with a "wait until idle and then 
free" to a model of "count users and free on last use", and I could easily 
have screwed up something.

"It works for me"(tm), including a shutdown/reboot cycle. 

Sergey, mind testing? You seem to be very good at consistently triggering 
odd things in the tty layer that few other people seem to ever hit.

Greg - I've signed off on these, but I wasn't planning on committing them 
to my master branch. So perhaps you could do these as the new tty 
maintainer, assuming we get an ack from Alan and testing by Sergey.

		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/