pick_link(): pass it struct path already with normal refcounting rules
authorAl Viro <viro@zeniv.linux.org.uk>
Tue, 3 Mar 2020 15:14:30 +0000 (10:14 -0500)
committerAl Viro <viro@zeniv.linux.org.uk>
Thu, 2 Apr 2020 05:09:23 +0000 (01:09 -0400)
step_into() tries to avoid grabbing and dropping mount references
on the steps that do not involve crossing mountpoints (which is
obviously the majority of cases).  So it uses a local struct path
with unusual refcounting rules - path.mnt is pinned if and only if
it's not equal to nd->path.mnt.

We used to have similar beasts all over the place and we had quite
a few bugs crop up in their handling - it's easy to get confused
when changing e.g. cleanup on failure exits (or adding a new check,
etc.)

Now that's mostly gone - the step_into() instance (which is what
we need them for) is the only one left.  It is exposed to mount
traversal and it's (shortly) seen by pick_link().  Since pick_link()
needs to store it in link stack, where the normal rules apply,
it has to make sure that mount is pinned regardless of nd->path.mnt
value.  That's done on all calls of pick_link() and very early
in those.  Let's do that in the caller (step_into()) instead -
that way the fewer places need to be aware of such struct path
instances.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
fs/namei.c

index 1f092f9e321b703785822e550bcea468940fcb1a..aab4eee6d56ae3877e259941731fe7c0b9422249 100644 (file)
@@ -1602,13 +1602,10 @@ static const char *pick_link(struct nameidata *nd, struct path *link,
        int error;
 
        if (unlikely(nd->total_link_count++ >= MAXSYMLINKS)) {
-               path_to_nameidata(link, nd);
+               if (!(nd->flags & LOOKUP_RCU))
+                       path_put(link);
                return ERR_PTR(-ELOOP);
        }
-       if (!(nd->flags & LOOKUP_RCU)) {
-               if (link->mnt == nd->path.mnt)
-                       mntget(link->mnt);
-       }
        error = nd_alloc_stack(nd);
        if (unlikely(error)) {
                if (error == -ECHILD) {
@@ -1713,10 +1710,13 @@ static const char *step_into(struct nameidata *nd, int flags,
                nd->seq = seq;
                return NULL;
        }
-       /* make sure that d_is_symlink above matches inode */
        if (nd->flags & LOOKUP_RCU) {
+               /* make sure that d_is_symlink above matches inode */
                if (read_seqcount_retry(&path.dentry->d_seq, seq))
                        return ERR_PTR(-ECHILD);
+       } else {
+               if (path.mnt == nd->path.mnt)
+                       mntget(path.mnt);
        }
        return pick_link(nd, &path, inode, seq, flags);
 }