linux-kernel - Re: [PATCH 01/13] devpts: Teach /dev/ptmx to find the associated devpts via path lookup

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]

Date:	Mon, 11 Apr 2016 21:16:41 -0500
From:	ebiederm@...ssion.com (Eric W. Biederman)
To:	Al Viro <viro@...IV.linux.org.uk>
Cc:	Linus Torvalds <torvalds@...ux-foundation.org>,
	"H. Peter Anvin" <hpa@...or.com>,
	Andy Lutomirski <luto@...capital.net>, security@...ian.org,
	"security\@kernel.org" <security@...nel.org>,
	"security\@ubuntu.com \>\> security" <security@...ntu.com>,
	Peter Hurley <peter@...leysoftware.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>, Greg KH <greg@...ah.com>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Jiri Slaby <jslaby@...e.com>, Florian Weimer <fw@...eb.enyo.de>
Subject: Re: [PATCH 01/13] devpts: Teach /dev/ptmx to find the associated devpts via path lookup

Al Viro <viro@...IV.linux.org.uk> writes:

> On Mon, Apr 11, 2016 at 07:10:47PM -0500, Eric W. Biederman wrote:
>> Actually for me this is about keeping the semantics simpler, and coming
>> up with a higher performance implementation.
>> 
>> A dentry that does an automount is already well defined.
>> 
>> Making the rule that accessing /dev/ptmx causes an automount of
>> /dev/pts/ptmx on top of the device node at /dev/ptmx is really simple,
>> with no special games.  It also makes it more obvious to userspace what
>> is going on.  AKA allows userspace to know which superblock does an open
>> ptmx master tty belongs to (and it happens in a backwards and forwards
>> compatible way).
>
> _What_ dentry?  Which filesystem would that be done to?  Whatever we have
> on /dev?  Or we suddenly get the fucking dentry operations change when
> dentry is attached to magical cdev inode?

Which dentry?  Any dentry that corresponds to the /dev/ptmx inode.
No filesystem changes just magic in init_special_inode that I have
not completely figured out yet.

If we can get an automount method in follow_automount from somewhere
cdev specific then a cdev can perform an automount comparitively
cleanly.  file_operations is attractive I am have not yet figured out a
clean method for passing the automount method yet.

For my proof of concept I am hardcoding things based on i_rdev.  Ugly
but servicable for testing out the idea.

A snip of my proof of concept code that seems to be working:

diff --git a/fs/inode.c b/fs/inode.c
index 69b8b526c194..d3de77b01a84 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -18,6 +18,7 @@
 #include <linux/buffer_head.h> /* for inode_has_buffers */
 #include <linux/ratelimit.h>
 #include <linux/list_lru.h>
+#include <linux/devpts_fs.h>
 #include <trace/events/writeback.h>
 #include "internal.h"
 
@@ -1917,6 +1918,11 @@ void init_special_inode(struct inode *inode, umode_t mode, dev_t rdev)
        if (S_ISCHR(mode)) {
                inode->i_fop = &def_chr_fops;
                inode->i_rdev = rdev;
+#if CONFIG_UNIX98_PTYS
+               if (rdev == MKDEV(TTYAUX_MAJOR, PTMX_MINOR)) {
+                       inode->i_flags |= S_AUTOMOUNT;
+               }
+#endif
        } else if (S_ISBLK(mode)) {
                inode->i_fop = &def_blk_fops;
                inode->i_rdev = rdev;

diff --git a/fs/namei.c b/fs/namei.c
index afb5137ca199..8894cf5fb43e 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -35,6 +35,8 @@
 #include <linux/fs_struct.h>
 #include <linux/posix_acl.h>
 #include <linux/hash.h>
+#include <linux/devpts_fs.h>
+#include <linux/major.h>
 #include <asm/uaccess.h>
 
 #include "internal.h"
@@ -1087,10 +1089,19 @@ EXPORT_SYMBOL(follow_up);
 static int follow_automount(struct path *path, struct nameidata *nd,
                            bool *need_mntput)
 {
+       struct vfsmount *(*automount)(struct path *) = NULL;
        struct vfsmount *mnt;
        int err;
 
-       if (!path->dentry->d_op || !path->dentry->d_op->d_automount)
+       if (path->dentry->d_op)
+               automount = path->dentry->d_op->d_automount;
+#if CONFIG_UNIX98_PTYS
+       if (path->dentry->d_inode &&
+           path->dentry->d_inode->i_rdev == MKDEV(TTYAUX_MAJOR, PTMX_MINOR)) {
+               automount = ptmx_automount;
+       }
+#endif
+       if (!automount)
                return -EREMOTE;
 
        /* We don't want to mount if someone's just doing a stat -
@@ -1113,7 +1124,7 @@ static int follow_automount(struct path *path, struct nameidata *nd,
        if (nd->total_link_count >= 40)
                return -ELOOP;
 
-       mnt = path->dentry->d_op->d_automount(path);
+       mnt = automount(path);
        if (IS_ERR(mnt)) {
                /*
                 * The filesystem is allowed to return -EISDIR here to indicate