Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753632AbcDTAY0 (ORCPT ); Tue, 19 Apr 2016 20:24:26 -0400 Received: from mail-pa0-f46.google.com ([209.85.220.46]:36094 "EHLO mail-pa0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752244AbcDTAYZ (ORCPT ); Tue, 19 Apr 2016 20:24:25 -0400 Subject: Re: [PATCH 01/16] devpts: Attempting to get it right To: Linus Torvalds , "Eric W. Biederman" References: <878u0s3orx.fsf_-_@x220.int.ebiederm.org> <20160409140909.42315e6d@lxorguk.ukuu.org.uk> <83FE8CD2-C0A2-4ADB-AEBD-8DD89AD4F88A@zytor.com> <87bn5ij0x1.fsf@x220.int.ebiederm.org> <78205895-E11D-417F-91DC-4BCA0B61A122@zytor.com> <570D4781.3070600@zytor.com> <877ffyzy1j.fsf_-_@x220.int.ebiederm.org> <87twixgsnq.fsf@x220.int.ebiederm.org> Cc: "H. Peter Anvin" , Andy Lutomirski , security@debian.org, "security@kernel.org" , Al Viro , "security@ubuntu.com >> security" , Serge Hallyn , Willy Tarreau , Aurelien Jarno , One Thousand Gnomes , Jann Horn , Greg KH , Linux Kernel Mailing List , Jiri Slaby , Florian Weimer From: Peter Hurley Message-ID: <5716CC33.8070302@hurleysoftware.com> Date: Tue, 19 Apr 2016 17:24:19 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1760 Lines: 38 On 04/19/2016 04:35 PM, Linus Torvalds wrote: > On Tue, Apr 19, 2016 at 3:06 PM, Eric W. Biederman > wrote: >> >> I have work inspired by this rolled into my code. I will post shortly >> after a little more testing. > > Actually, I have a slightly fixed version in my tree. I've been > running this on my own machines for a few days, just to verify, along > with some testing. > > The fixes are some cleanups of the header file (the !UNIX98 section > that nobody uses was bogus), and fixing "devpts_get_ref()" to get the > "struct file *" argument too. The current code doesn't need it, but > the code to actually look up the right pts/ directory from the ptmx > file open needs it because that's where the path is - passing in just > the inode isn't sufficient). > > Anyway, I think I'll just merge my branch instead of sending out > another emailed patch, because I don't think that patch is > controversial or unsafe. It doesn't actually change any semantics, and > only does cleanups. If it breaks something due to the rules about > private_data being different for slave and master side pty's, then the > old code was broken too - it used to always put a inode pointer in > there, but they were very different inode pointers. Yes, the old code was broken. The new code always uses the ptmx inode. Which is a stopgap measure to prevent the devpts instance from teardown when the only open filp is /dev/tty. What I want to do instead (but not for -stable) is bump the inode reference at controlling tty association instead (either first qualifying open or ioctl(TIOCSCTTY)). That way it would always be the slave inode. This method is non-trivial though because it makes disassociation more complicated. Regards, Peter Hurley