namei: Fix use after free in kern_path_locked
authorStephen Brennan <stephen.s.brennan@oracle.com>
Wed, 1 Sep 2021 17:51:41 +0000 (10:51 -0700)
committerAl Viro <viro@zeniv.linux.org.uk>
Tue, 7 Sep 2021 19:56:16 +0000 (15:56 -0400)
In 0ee50b47532a ("namei: change filename_parentat() calling
conventions"), filename_parentat() was made to always call putname() on
the  filename before returning, and kern_path_locked() was migrated to
this calling convention.  However, kern_path_locked() uses the "last"
parameter to lookup and potentially create a new dentry. The last
parameter contains the last component of the path and points within the
filename, which was recently freed at the end of filename_parentat().
Thus, when kern_path_locked() calls __lookup_hash(), it is using the
filename after it has already been freed.

In other words, these calling conventions had been wrong for the
only remaining caller of filename_parentat().  Everything else
is using __filename_parentat(), which does not drop the reference;
so should kern_path_locked().

Switch kern_path_locked() to use of __filename_parentat() and move
getting/dropping struct filename into wrapper.  Remove filename_parentat(),
now that we have no remaining callers.

Fixes: 0ee50b47532a ("namei: change filename_parentat() calling conventions")
Link: https://lore.kernel.org/linux-fsdevel/YS9D4AlEsaCxLFV0@infradead.org/
Link: https://lore.kernel.org/linux-fsdevel/YS+csMTV2tTXKg3s@zeniv-ca.linux.org.uk/
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Reported-by: syzbot+fb0d60a179096e8c2731@syzkaller.appspotmail.com
Signed-off-by: Stephen Brennan <stephen.s.brennan@oracle.com>
Co-authored-by: Dmitry Kadashev <dkadashev@gmail.com>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
fs/namei.c

index 95a881e..33a2a85 100644 (file)
@@ -2514,9 +2514,10 @@ static int path_parentat(struct nameidata *nd, unsigned flags,
        return err;
 }
 
+/* Note: this does not consume "name" */
 static int __filename_parentat(int dfd, struct filename *name,
-                               unsigned int flags, struct path *parent,
-                               struct qstr *last, int *type)
+                            unsigned int flags, struct path *parent,
+                            struct qstr *last, int *type)
 {
        int retval;
        struct nameidata nd;
@@ -2538,25 +2539,14 @@ static int __filename_parentat(int dfd, struct filename *name,
        return retval;
 }
 
-static int filename_parentat(int dfd, struct filename *name,
-                               unsigned int flags, struct path *parent,
-                               struct qstr *last, int *type)
-{
-       int retval = __filename_parentat(dfd, name, flags, parent, last, type);
-
-       putname(name);
-       return retval;
-}
-
 /* does lookup, returns the object with parent locked */
-struct dentry *kern_path_locked(const char *name, struct path *path)
+static struct dentry *__kern_path_locked(struct filename *name, struct path *path)
 {
        struct dentry *d;
        struct qstr last;
        int type, error;
 
-       error = filename_parentat(AT_FDCWD, getname_kernel(name), 0, path,
-                                   &last, &type);
+       error = __filename_parentat(AT_FDCWD, name, 0, path, &last, &type);
        if (error)
                return ERR_PTR(error);
        if (unlikely(type != LAST_NORM)) {
@@ -2572,6 +2562,15 @@ struct dentry *kern_path_locked(const char *name, struct path *path)
        return d;
 }
 
+struct dentry *kern_path_locked(const char *name, struct path *path)
+{
+       struct filename *filename = getname_kernel(name);
+       struct dentry *res = __kern_path_locked(filename, path);
+
+       putname(filename);
+       return res;
+}
+
 int kern_path(const char *name, unsigned int flags, struct path *path)
 {
        return filename_lookup(AT_FDCWD, getname_kernel(name),