diff options
author | Stephen Brennan <stephen.s.brennan@oracle.com> | 2021-09-01 10:51:41 -0700 |
---|---|---|
committer | Al Viro <viro@zeniv.linux.org.uk> | 2021-09-07 15:56:16 -0400 |
commit | 0766ec82e5fb26fc5dc6d592bc61865608bdc651 (patch) | |
tree | ddc2d8422eab2d84d23f6ac91e8a3a89b15cee36 | |
parent | 4b93c544e90e2b28326182d31ee008eb80e02074 (diff) | |
download | linux-0766ec82e5fb26fc5dc6d592bc61865608bdc651.tar.bz2 |
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 <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>
-rw-r--r-- | fs/namei.c | 29 |
1 files 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), |