linux-kernel - Re: [PATCH 1/2 v2] pty: fix possible use after free of tty->driver

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: <20160114200946.GC3036@dhcppc10.redhat.com>
Date:	Thu, 14 Jan 2016 18:09:47 -0200
From:	"Herton R. Krzesinski" <herton@...hat.com>
To:	Josh Triplett <josh@...htriplett.org>
Cc:	Peter Hurley <peter@...leysoftware.com>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	linux-kernel@...r.kernel.org, Alan Cox <alan@...ux.intel.com>,
	Jiri Slaby <jslaby@...e.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Al Viro <viro@...iv.linux.org.uk>,
	David Howells <dhowells@...hat.com>
Subject: Re: [PATCH 1/2 v2] pty: fix possible use after free of
 tty->driver_data

On Wed, Jan 13, 2016 at 10:28:44AM -0800, Josh Triplett wrote:
> On Wed, Jan 13, 2016 at 09:39:29AM -0800, Peter Hurley wrote:
> > On 01/11/2016 06:07 AM, Herton R. Krzesinski wrote:
> > > This change fixes a bug for a corner case where we have the the last
> > > release from a pty master/slave coming from a previously opened /dev/tty
> > > file. When this happens, the tty->driver_data can be stale, due to all
> > > ptmx or pts/N files having already been closed before (and thus the inode
> > > related to these files, which tty->driver_data points to, being already
> > > freed/destroyed).
> > > 
> > > The fix here is to keep a reference on the opened master ptmx inode.
> > > We maintain the inode referenced until the final pty_unix98_shutdown,
> > > and only pass this inode to devpts_kill_index.
> > 
> > Ideally, the tty core should be bumping the inode count for the underlying
> > controlling tty
> 
> That does indeed sound like the right fix.  /dev/tty doesn't act exactly
> like opening the underlying device (as it also supports the TIOCNOTTY
> ioctl), but it should definitely hold a reference to that underlying
> device.

Yeah, I thought previously as well that this should go to tty core (my thinking
though was to get extra references to the opened files instead of the inode).

However, what inode should I choose to increment the reference count in the
tty->tty_files list? I know most cases the device file will always be at the
/dev, but if it's opened from another place/file?

And handling this would overcomplicate a case which is pty specific, unless
I miss something here pty is the only inode user, and seem not worth or
useful to have at the moment this at tty core.

thanks,
-- 
Herton.