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

[Eric W. Biederman]

Al Viro <v...@zeniv.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