From 0766ec82e5fb26fc5dc6d592bc61865608bdc651 Mon Sep 17 00:00:00 2001 From: Stephen Brennan Date: Wed, 1 Sep 2021 10:51:41 -0700 Subject: namei: Fix use after free in kern_path_locked 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 Cc: Al Viro Reported-by: syzbot+fb0d60a179096e8c2731@syzkaller.appspotmail.com Signed-off-by: Stephen Brennan Co-authored-by: Dmitry Kadashev Signed-off-by: Al Viro --- fs/namei.c | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/fs/namei.c b/fs/namei.c index 95a881e0552b..33a2a8504099 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -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), -- cgit v1.2.3