[PATCH 1/3] namei: Ensure nd->path.{dentry,mnt} are always valid pointers

[Al Viro]

Initialize them in set_nameidata() and make sure that terminate_walk()
clears them once the pointers become potentially invalid (i.e.
we leave RCU mode or drop them in non-RCU one).  Currently we have
"path_init() always initializes them and nobody accesses them outside of
path_init()/terminate_walk() segments", which is asking for trouble.

With that change we would have nd->path.{mnt,dentry}
1) always valid - NULL or pointing to currently allocated objects.
2) non-NULL while we are successfully walking
3) NULL when we are not walking at all
4) contributing to refcounts whenever non-NULL outside of RCU mode.

Fixes: 6c6ec2b0a3e0 ("fs: add support for LOOKUP_CACHED")
Reported-by: syzbot+c88a7030da47945a3cc3@xxxxxxxxxxxxxxxxxxxxxxxxx
Cc: stable@xxxxxxxxxxxxxxx
Cc: linux-fsdevel@xxxxxxxxxxxxxxx
Signed-off-by: Al Viro <viro@xxxxxxxxxxxxxxxxxx>
Signed-off-by: Christian Brauner <christian.brauner@xxxxxxxxxx>
---
 fs/namei.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index 216f16e74351..618055419da3 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -573,9 +573,18 @@ struct nameidata {
 	umode_t		dir_mode;
 } __randomize_layout;
 
+static inline void clear_nameidata(struct nameidata *nd)
+{
+	nd->depth = 0;
+	nd->path.mnt = NULL;
+	nd->path.dentry = NULL;
+}
+
 static void set_nameidata(struct nameidata *p, int dfd, struct filename *name)
 {
 	struct nameidata *old = current->nameidata;
+
+	clear_nameidata(p);
 	p->stack = p->internal;
 	p->dfd = dfd;
 	p->name = name;
@@ -651,7 +660,7 @@ static void terminate_walk(struct nameidata *nd)
 		nd->flags &= ~LOOKUP_RCU;
 		rcu_read_unlock();
 	}
-	nd->depth = 0;
+	clear_nameidata(nd);
 }
 
 /* path_put is needed afterwards regardless of success or failure */
@@ -2322,8 +2331,6 @@ static const char *path_init(struct nameidata *nd, unsigned flags)
 	}
 
 	nd->root.mnt = NULL;
-	nd->path.mnt = NULL;
-	nd->path.dentry = NULL;
 
 	/* Absolute pathname -- fetch the root (LOOKUP_IN_ROOT uses nd->dfd). */
 	if (*s == '/' && !(flags & LOOKUP_IN_ROOT)) {
-- 
2.27.0



[1]:
diff --git a/fs/namei.c b/fs/namei.c
index ae1b5cd7b1a9..572a0d6e22cc 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -2424,33 +2424,49 @@ static int path_lookupat(struct nameidata *nd, unsigned flags, struct path *path
        int err;

        s = path_init(nd, flags);
-       if (IS_ERR(s))
-               return PTR_ERR(s);
+       if (IS_ERR(s)) {
+               err = PTR_ERR(s);
+               goto out_terminate;
+       }

        if (unlikely(flags & LOOKUP_DOWN)) {
                err = handle_lookup_down(nd);
                if (unlikely(err < 0))
-                       s = ERR_PTR(err);
+                       goto out_terminate;
        }

-       while (!(err = link_path_walk(s, nd)) &&
-              (s = lookup_last(nd)) != NULL)
-               ;
-       if (!err)
-               err = complete_walk(nd);
+       for (;;) {
+               err = link_path_walk(s, nd);
+               if (err)
+                       goto out_terminate;

-       if (!err && nd->flags & LOOKUP_DIRECTORY)
-               if (!d_can_lookup(nd->path.dentry))
-                       err = -ENOTDIR;
-       if (!err && unlikely(nd->flags & LOOKUP_MOUNTPOINT)) {
+               s = lookup_last(nd);
+               if (!s)
+                       break;
+       }
+
+       err = complete_walk(nd);
+       if (err)
+               goto out_terminate;
+
+       if ((nd->flags & LOOKUP_DIRECTORY) &&
+           (!d_can_lookup(nd->path.dentry))) {
+               err = -ENOTDIR;
+               goto out_terminate;
+       }
+
+       if (unlikely(nd->flags & LOOKUP_MOUNTPOINT)) {
                err = handle_lookup_down(nd);
                nd->flags &= ~LOOKUP_JUMPED; // no d_weak_revalidate(), please...
+               if (err)
+                       goto out_terminate;
        }
-       if (!err) {
-               *path = nd->path;
-               nd->path.mnt = NULL;
-               nd->path.dentry = NULL;
-       }
+
+       *path = nd->path;
+       nd->path.mnt = NULL;
+       nd->path.dentry = NULL;
+
+out_terminate:
        terminate_walk(nd);
        return err;
 }