procfs: get rid of ancient BS in pid_revalidate() uses
authorAl Viro <viro@zeniv.linux.org.uk>
Thu, 3 May 2018 01:26:16 +0000 (21:26 -0400)
committerAl Viro <viro@zeniv.linux.org.uk>
Tue, 22 May 2018 18:28:03 +0000 (14:28 -0400)
First of all, calling pid_revalidate() in the end of <pid>/* lookups
is *not* about closing any kind of races; that used to be true once
upon a time, but these days those comments are actively misleading.
Especially since pid_revalidate() doesn't even do d_drop() on
failure anymore.  It doesn't matter, anyway, since once
pid_revalidate() starts returning false, ->d_delete() of those
dentries starts saying "don't keep"; they won't get stuck in
dcache any longer than they are pinned.

These calls cannot be just removed, though - the side effect of
pid_revalidate() (updating i_uid/i_gid/etc.) is what we are calling
it for here.

Let's separate the "update ownership" into a new helper (pid_update_inode())
and use it, both in lookups and in pid_revalidate() itself.

The comments in pid_revalidate() are also out of date - they refer to
the time when pid_revalidate() used to call d_drop() directly...

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
fs/proc/base.c
fs/proc/internal.h
fs/proc/namespaces.c

index eafa39a3a88cb479eaca9d164e05cc4dbca1db70..6e0875505898f7f8e8b51a214e04ed8f9510f4e7 100644 (file)
@@ -1803,15 +1803,22 @@ int pid_getattr(const struct path *path, struct kstat *stat,
 /* dentry stuff */
 
 /*
- *     Exceptional case: normally we are not allowed to unhash a busy
- * directory. In this case, however, we can do it - no aliasing problems
- * due to the way we treat inodes.
- *
+ * Set <pid>/... inode ownership (can change due to setuid(), etc.)
+ */
+void pid_update_inode(struct task_struct *task, struct inode *inode)
+{
+       task_dump_owner(task, inode->i_mode, &inode->i_uid, &inode->i_gid);
+
+       inode->i_mode &= ~(S_ISUID | S_ISGID);
+       security_task_to_inode(task, inode);
+}
+
+/*
  * Rewrite the inode's ownerships here because the owning task may have
  * performed a setuid(), etc.
  *
  */
-int pid_revalidate(struct dentry *dentry, unsigned int flags)
+static int pid_revalidate(struct dentry *dentry, unsigned int flags)
 {
        struct inode *inode;
        struct task_struct *task;
@@ -1823,10 +1830,7 @@ int pid_revalidate(struct dentry *dentry, unsigned int flags)
        task = get_proc_task(inode);
 
        if (task) {
-               task_dump_owner(task, inode->i_mode, &inode->i_uid, &inode->i_gid);
-
-               inode->i_mode &= ~(S_ISUID | S_ISGID);
-               security_task_to_inode(task, inode);
+               pid_update_inode(task, inode);
                put_task_struct(task);
                return 1;
        }
@@ -2438,7 +2442,7 @@ static int proc_pident_instantiate(struct inode *dir,
 
        inode = proc_pid_make_inode(dir->i_sb, task, p->mode);
        if (!inode)
-               goto out;
+               return -ENOENT;
 
        ei = PROC_I(inode);
        if (S_ISDIR(inode->i_mode))
@@ -2448,13 +2452,10 @@ static int proc_pident_instantiate(struct inode *dir,
        if (p->fop)
                inode->i_fop = p->fop;
        ei->op = p->op;
+       pid_update_inode(task, inode);
        d_set_d_op(dentry, &pid_dentry_operations);
        d_add(dentry, inode);
-       /* Close the race of the process dying before we return the dentry */
-       if (pid_revalidate(dentry, 0))
-               return 0;
-out:
-       return -ENOENT;
+       return 0;
 }
 
 static struct dentry *proc_pident_lookup(struct inode *dir, 
@@ -3140,22 +3141,18 @@ static int proc_pid_instantiate(struct inode *dir,
 
        inode = proc_pid_make_inode(dir->i_sb, task, S_IFDIR | S_IRUGO | S_IXUGO);
        if (!inode)
-               goto out;
+               return -ENOENT;
 
        inode->i_op = &proc_tgid_base_inode_operations;
        inode->i_fop = &proc_tgid_base_operations;
        inode->i_flags|=S_IMMUTABLE;
 
        set_nlink(inode, nlink_tgid);
+       pid_update_inode(task, inode);
 
        d_set_d_op(dentry, &pid_dentry_operations);
-
        d_add(dentry, inode);
-       /* Close the race of the process dying before we return the dentry */
-       if (pid_revalidate(dentry, 0))
-               return 0;
-out:
-       return -ENOENT;
+       return 0;
 }
 
 struct dentry *proc_pid_lookup(struct inode *dir, struct dentry * dentry, unsigned int flags)
@@ -3434,23 +3431,19 @@ static int proc_task_instantiate(struct inode *dir,
 {
        struct inode *inode;
        inode = proc_pid_make_inode(dir->i_sb, task, S_IFDIR | S_IRUGO | S_IXUGO);
-
        if (!inode)
-               goto out;
+               return -ENOENT;
+
        inode->i_op = &proc_tid_base_inode_operations;
        inode->i_fop = &proc_tid_base_operations;
-       inode->i_flags|=S_IMMUTABLE;
+       inode->i_flags |= S_IMMUTABLE;
 
        set_nlink(inode, nlink_tid);
+       pid_update_inode(task, inode);
 
        d_set_d_op(dentry, &pid_dentry_operations);
-
        d_add(dentry, inode);
-       /* Close the race of the process dying before we return the dentry */
-       if (pid_revalidate(dentry, 0))
-               return 0;
-out:
-       return -ENOENT;
+       return 0;
 }
 
 static struct dentry *proc_task_lookup(struct inode *dir, struct dentry * dentry, unsigned int flags)
index 0f1692e63cb62324f69a64100daae0bdfc383784..04a455b9ae6975b90da4eab326855efb4c2a4650 100644 (file)
@@ -147,7 +147,7 @@ extern const struct dentry_operations pid_dentry_operations;
 extern int pid_getattr(const struct path *, struct kstat *, u32, unsigned int);
 extern int proc_setattr(struct dentry *, struct iattr *);
 extern struct inode *proc_pid_make_inode(struct super_block *, struct task_struct *, umode_t);
-extern int pid_revalidate(struct dentry *, unsigned int);
+extern void pid_update_inode(struct task_struct *, struct inode *);
 extern int pid_delete_dentry(const struct dentry *);
 extern int proc_pid_readdir(struct file *, struct dir_context *);
 extern struct dentry *proc_pid_lookup(struct inode *, struct dentry *, unsigned int);
index 59b17e509f4633c31d86fa34425838bc390a6a88..ad1adce6541dbef7bbc21bbe983557548bc0bbff 100644 (file)
@@ -96,19 +96,16 @@ static int proc_ns_instantiate(struct inode *dir,
 
        inode = proc_pid_make_inode(dir->i_sb, task, S_IFLNK | S_IRWXUGO);
        if (!inode)
-               goto out;
+               return -ENOENT;
 
        ei = PROC_I(inode);
        inode->i_op = &proc_ns_link_inode_operations;
        ei->ns_ops = ns_ops;
+       pid_update_inode(task, inode);
 
        d_set_d_op(dentry, &pid_dentry_operations);
        d_add(dentry, inode);
-       /* Close the race of the process dying before we return the dentry */
-       if (pid_revalidate(dentry, 0))
-               return 0;
-out:
-       return -ENOENT;
+       return 0;
 }
 
 static int proc_ns_dir_readdir(struct file *file, struct dir_context *ctx)