From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932806AbcFBUeP (ORCPT ); Thu, 2 Jun 2016 16:34:15 -0400 Received: from out02.mta.xmission.com ([166.70.13.232]:46587 "EHLO out02.mta.xmission.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932171AbcFBUeM (ORCPT ); Thu, 2 Jun 2016 16:34:12 -0400 From: ebiederm@xmission.com (Eric W. Biederman) To: Linus Torvalds Cc: Greg KH , Peter Hurley , "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 , Linux Kernel Mailing List , Jiri Slaby , Florian Weimer , Konstantin Khlebnikov References: <87oa95gevf.fsf_-_@x220.int.ebiederm.org> <87mvoo8h3d.fsf@x220.int.ebiederm.org> <87fuug5i6r.fsf_-_@x220.int.ebiederm.org> <874mabt3df.fsf_-_@x220.int.ebiederm.org> <20160506193547.GA30024@kroah.com> <572CF43D.2020708@hurleysoftware.com> <20160506195459.GB27790@kroah.com> <878tyn4nis.fsf_-_@x220.int.ebiederm.org> Date: Thu, 02 Jun 2016 15:22:00 -0500 In-Reply-To: (Linus Torvalds's message of "Thu, 2 Jun 2016 11:57:40 -0700") Message-ID: <87porzuys7.fsf@x220.int.ebiederm.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-XM-AID: U2FsdGVkX18GD28+6dHO/4AUgJwxiPNO0Gt7tvEpuUo= X-SA-Exim-Connect-IP: 67.3.226.120 X-SA-Exim-Mail-From: ebiederm@xmission.com X-Spam-Report: * -1.0 ALL_TRUSTED Passed through trusted hosts only via SMTP * 0.7 XMSubLong Long Subject * 0.0 TVD_RCVD_IP Message was received from an IP address * 0.0 T_TM2_M_HEADER_IN_MSG BODY: No description available. * 0.8 BAYES_50 BODY: Bayes spam probability is 40 to 60% * [score: 0.5000] * -0.0 DCC_CHECK_NEGATIVE Not listed in DCC * [sa06 1397; Body=1 Fuz1=1 Fuz2=1] * 0.0 T_TooManySym_01 4+ unique symbols in subject X-Spam-DCC: XMission; sa06 1397; Body=1 Fuz1=1 Fuz2=1 X-Spam-Combo: ;Linus Torvalds X-Spam-Relay-Country: X-Spam-Timing: total 369 ms - load_scoreonly_sql: 0.05 (0.0%), signal_user_changed: 7 (1.9%), b_tie_ro: 6 (1.7%), parse: 0.75 (0.2%), extract_message_metadata: 19 (5.1%), get_uri_detail_list: 2.7 (0.7%), tests_pri_-1000: 10 (2.8%), tests_pri_-950: 1.18 (0.3%), tests_pri_-900: 1.04 (0.3%), tests_pri_-400: 26 (7.2%), check_bayes: 25 (6.9%), b_tokenize: 7 (2.0%), b_tok_get_all: 8 (2.3%), b_comp_prob: 2.1 (0.6%), b_tok_touch_all: 4.7 (1.3%), b_finish: 0.67 (0.2%), tests_pri_0: 295 (80.0%), check_dkim_signature: 0.75 (0.2%), check_dkim_adsp: 3.2 (0.9%), tests_pri_500: 4.1 (1.1%), rewrite_mail: 0.00 (0.0%) Subject: Re: [PATCH tty-next] devpts: Make each mount of devpts an independent filesystem. X-Spam-Flag: No X-SA-Exim-Version: 4.2.1 (built Thu, 05 May 2016 13:38:54 -0600) X-SA-Exim-Scanned: Yes (on in02.mta.xmission.com) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Linus Torvalds writes: > On Thu, Jun 2, 2016 at 8:29 AM, Eric W. Biederman wrote: >> >> The /dev/ptmx device node is changed to lookup the directory entry >> "pts" in the same directory as the /dev/ptmx device node was opened >> in. If there is a "pts" entry and that entry is a devpts filesystem >> /dev/ptmx uses that filesystem. Otherwise the open of /dev/ptmx >> fails. >> >> The DEVPTS_MULTIPLE_INSTANCES configuration option is removed, >> so that userspace can now safely depend on each mount of devpts >> creating a new instance of the filesystem. >> >> Each mount of devpts is now a separate and equal filesystem. > > Ok, this came outside the merge window, but the patches predate it by > a lot, so I'm actually inclined to finally get this all sorted out and > just apply it. > > Al, I'm not seeing anything horribly questionable in the path_pts() > function, although I think it should just do lookup_one_len_unlocked() > and follow_mount() instead of open-coding that hashing etc. So I > actually prefer the ptmx_to_pts() function I posted back in April: > The problem with lookup_one_len_unlocked is that it still calls inode_permission. As per previous discussions we don't want the path based permission checks involved in that lookup. I also have a path_connected check after the dget_parent to ensure there is not a crazy situation involved where the parent directory of the ptmx dentry is not connected to it's mount. Plus my version by not having the lookup_slow fallback that is in lookup_one_len_unlocked never has to even consider the inode mutex or any of that, so it is fundamentally simpler to deal with. And of course we are dealing with something that must be a mount point which pins the dentry, which makes a lookup_slow fallback pointless. > static struct dentry *ptmx_to_pts(struct path *ptmx) > { > struct dentry *dev = dget_parent(ptmx->dentry); > > if (dev) { > struct path path; > > path.dentry = lookup_one_len_unlocked("pts", dev, 3); > if (path.dentry) { ^^^^^^^^^^^^^^^ That check is buggy because it needs at least an IS_ERR(path.dentry) > path.mnt = mntget(ptmx->mnt); > follow_mount(&path); > mntput(path.mnt); > return path.dentry; > } > } > return NULL; > } > > but I don't care *too* much. Al? > > Comments? > > Linus Eric