Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754337AbbLKXBM (ORCPT ); Fri, 11 Dec 2015 18:01:12 -0500 Received: from mail-ob0-f177.google.com ([209.85.214.177]:36209 "EHLO mail-ob0-f177.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752252AbbLKXBK (ORCPT ); Fri, 11 Dec 2015 18:01:10 -0500 MIME-Version: 1.0 In-Reply-To: <20151211225825.GA24676@pc.thejh.net> References: <20151209013859.GA12442@kroah.com> <20151209083225.GA30452@1wt.eu> <87wpskyds7.fsf_-_@x220.int.ebiederm.org> <20151211210400.GL20997@ZenIV.linux.org.uk> <87h9jovgf7.fsf@x220.int.ebiederm.org> <566B4931.5080806@zytor.com> <87y4d0sjff.fsf@x220.int.ebiederm.org> <20151211225825.GA24676@pc.thejh.net> From: Andy Lutomirski Date: Fri, 11 Dec 2015 15:00:49 -0800 Message-ID: Subject: Re: [PATCH] devpts: Sensible /dev/ptmx & force newinstance To: Jann Horn Cc: "Eric W. Biederman" , "H. Peter Anvin" , Al Viro , Greg KH , Jiri Slaby , Linus Torvalds , Aurelien Jarno , Florian Weimer , Serge Hallyn , "security@kernel.org" , "security@ubuntu.com >> security" , security@debian.org, Willy Tarreau , "linux-kernel@vger.kernel.org" Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3138 Lines: 73 On Fri, Dec 11, 2015 at 2:58 PM, Jann Horn wrote: > On Fri, Dec 11, 2015 at 02:52:01PM -0800, Andy Lutomirski wrote: >> On Fri, Dec 11, 2015 at 2:35 PM, Eric W. Biederman >> wrote: >> > Andy Lutomirski writes: >> > >> >> On Fri, Dec 11, 2015 at 2:07 PM, H. Peter Anvin wrote: >> >>> On 12/11/15 13:48, Andy Lutomirski wrote: >> >>>> On Fri, Dec 11, 2015 at 1:11 PM, Eric W. Biederman >> >>>> wrote: >> >>>>> Al Viro writes: >> >>>>> >> >>>>>> On Fri, Dec 11, 2015 at 01:40:40PM -0600, Eric W. Biederman wrote: >> >>>>>> >> >>>>>>> + inode = path.dentry->d_inode; >> >>>>>>> + filp->f_path = path; >> >>>>>>> + filp->f_inode = inode; >> >>>>>>> + filp->f_mapping = inode->i_mapping; >> >>>>>>> + path_put(&old); >> >>>>>> >> >>>>>> Don't. You are creating a fairly subtle constraint on what the code in >> >>>>>> fs/open.c and fs/namei.c can do, for no good reason. You can bloody >> >>>>>> well maintain the information you need without that. >> >>>>> >> >>>>> There is a good reason. We can not write a race free version of ptsname >> >>>>> without it. >> >>>> >> >>>> As long as this is for new userspace code, would it make sense to just >> >>>> add a new ioctl to ask "does this ptmx fd match this /dev/pts fd?" >> >>>> >> >>> >> >>> For the newinstance case st_dev should match between the master and the >> >>> slave. Unfortunately this is not the case for a legacy ptmx, as a >> >>> stat() on the master descriptor still returns the st_dev, st_rdev, and >> >>> st_ino for the ptmx device node. >> >> >> >> Sure, but I'm not talking about stat. I'm saying that we could add a >> >> new ioctl that works on any ptmx fd (/dev/ptmx or /dev/pts/ptmx) that >> >> answers the question "does this ptmx logically belong to the given >> >> devpts filesystem". >> >> >> >> Since it's not stat, we can make it do whatever we want, including >> >> following a link to the devpts instance that isn't f_path or f_inode. >> > >> > The useful ioctl to add in my opinion would be one that actually opens >> > the slave, at which point ptsname could become ttyname, and that closes >> > races in grantpt. >> >> Unfortunately, ptsname is POSIX, so we can't get rid of it. It's a >> bad idea, but it's in the standard. > > But then ptsname could become "open the slave, call ttyname() on it, close > the slave". Unless opening the slave would have side effects? Hmm, fair enough. So maybe that does make sense after all. Anyway, I still think there are two pieces here: 1. Fix /dev/ptmx so that we can banish newinstance=0. 2. Fix libc. If that needs kernel help, then so be it. ISTM we could still implement the "open the slave" operation for (2) as an ioctl that does the appropriate magic the fd is /dev/ptmx as opposed to /dev/pts/ptmx. --Andy -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/