linux-kernel - Re: [PATCH tty-next] devpts: Make each mount of devpts an independent filesystem.

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] [day] [month] [year] [list]

Message-ID: <CA+55aFwAB6XkW6SJxrABKRpTEXrJo=W7kyv0CHrOLhmZ-22Tng@mail.gmail.com>
Date:	Thu, 2 Jun 2016 14:44:03 -0700
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	"Eric W. Biederman" <ebiederm@...ssion.com>
Cc:	"H. Peter Anvin" <hpa@...or.com>, Greg KH <greg@...ah.com>,
	Peter Hurley <peter@...leysoftware.com>,
	Andy Lutomirski <luto@...capital.net>, security@...ian.org,
	"security@...nel.org" <security@...nel.org>,
	Al Viro <viro@...iv.linux.org.uk>,
	"security@...ntu.com >> security" <security@...ntu.com>,
	Serge Hallyn <serge.hallyn@...ntu.com>,
	Willy Tarreau <w@....eu>,
	Aurelien Jarno <aurelien@...el32.net>,
	One Thousand Gnomes <gnomes@...rguk.ukuu.org.uk>,
	Jann Horn <jann@...jh.net>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Jiri Slaby <jslaby@...e.com>,
	Florian Weimer <fw@...eb.enyo.de>,
	Konstantin Khlebnikov <koct9i@...il.com>
Subject: Re: [PATCH tty-next] devpts: Make each mount of devpts an independent filesystem.

On Thu, Jun 2, 2016 at 2:23 PM, Eric W. Biederman <ebiederm@...ssion.com> wrote:
>
> We certainly don't need the permission check.

I agree that we don't need it, but it does simplify the code, and it
doesn't actually harm anything, since the thing we check for
permissions is the same /dev directory, which we already know we had
lookup permissions for for the original ptmx node anyway.

The permission checks I didn't like were the permission checks for the
pts/ and pts/ptmx nodes, because they had nothing to do with the
permission for the original node. In contrast, this is just about a
redundant permission check we already did, and that simplifies the
code a bit.

That said, you're right that it needs to use IS_ERR(path.dentry). And
the whole "lookup_one_len_unlocked()" simplification is just a small
detail that doesn't much matter, so it's not a big deal.

And it is arguable that "d_hash_and_lookup()" that you use is actually
simpler than "lookup_one_len_unlocked()", since we are really only
need to look into the dentry cache, and that's all that
d_hash_and_lookup() does (ie it only ever does a d_lookup(), while
lookup_one_len_unlocked() ends up doing a full lookup in the failuer
case that we don't even care about.

So I don't feel very strongly about it. Your patch is ok by me.

                Linus