Skip to content

Commit

Permalink
ksmbd: fix racy issue from using ->d_parent and ->d_name
Browse files Browse the repository at this point in the history
[ Upstream commit 74d7970 ]

Al pointed out that ksmbd has racy issue from using ->d_parent and ->d_name
in ksmbd_vfs_unlink and smb2_vfs_rename(). and use new lock_rename_child()
to lock stable parent while underlying rename racy.
Introduce vfs_path_parent_lookup helper to avoid out of share access and
export vfs functions like the following ones to use
vfs_path_parent_lookup().
 - rename __lookup_hash() to lookup_one_qstr_excl().
 - export lookup_one_qstr_excl().
 - export getname_kernel() and putname().

vfs_path_parent_lookup() is used for parent lookup of destination file
using absolute pathname given from FILE_RENAME_INFORMATION request.

Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
  • Loading branch information
namjaejeon authored and gregkh committed Jan 5, 2024
1 parent 6e99fbb commit 6927ffe
Show file tree
Hide file tree
Showing 6 changed files with 283 additions and 386 deletions.
57 changes: 45 additions & 12 deletions fs/namei.c
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,7 @@ getname_kernel(const char * filename)

return result;
}
EXPORT_SYMBOL(getname_kernel);

void putname(struct filename *name)
{
Expand All @@ -271,6 +272,7 @@ void putname(struct filename *name)
} else
__putname(name);
}
EXPORT_SYMBOL(putname);

/**
* check_acl - perform ACL permission checking
Expand Down Expand Up @@ -1581,8 +1583,9 @@ static struct dentry *lookup_dcache(const struct qstr *name,
* when directory is guaranteed to have no in-lookup children
* at all.
*/
static struct dentry *__lookup_hash(const struct qstr *name,
struct dentry *base, unsigned int flags)
struct dentry *lookup_one_qstr_excl(const struct qstr *name,
struct dentry *base,
unsigned int flags)
{
struct dentry *dentry = lookup_dcache(name, base, flags);
struct dentry *old;
Expand All @@ -1606,6 +1609,7 @@ static struct dentry *__lookup_hash(const struct qstr *name,
}
return dentry;
}
EXPORT_SYMBOL(lookup_one_qstr_excl);

static struct dentry *lookup_fast(struct nameidata *nd)
{
Expand Down Expand Up @@ -2532,16 +2536,17 @@ static int path_parentat(struct nameidata *nd, unsigned flags,
}

/* 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)
static int __filename_parentat(int dfd, struct filename *name,
unsigned int flags, struct path *parent,
struct qstr *last, int *type,
const struct path *root)
{
int retval;
struct nameidata nd;

if (IS_ERR(name))
return PTR_ERR(name);
set_nameidata(&nd, dfd, name, NULL);
set_nameidata(&nd, dfd, name, root);
retval = path_parentat(&nd, flags | LOOKUP_RCU, parent);
if (unlikely(retval == -ECHILD))
retval = path_parentat(&nd, flags, parent);
Expand All @@ -2556,6 +2561,13 @@ 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)
{
return __filename_parentat(dfd, name, flags, parent, last, type, NULL);
}

/* does lookup, returns the object with parent locked */
static struct dentry *__kern_path_locked(struct filename *name, struct path *path)
{
Expand All @@ -2571,7 +2583,7 @@ static struct dentry *__kern_path_locked(struct filename *name, struct path *pat
return ERR_PTR(-EINVAL);
}
inode_lock_nested(path->dentry->d_inode, I_MUTEX_PARENT);
d = __lookup_hash(&last, path->dentry, 0);
d = lookup_one_qstr_excl(&last, path->dentry, 0);
if (IS_ERR(d)) {
inode_unlock(path->dentry->d_inode);
path_put(path);
Expand Down Expand Up @@ -2599,6 +2611,24 @@ int kern_path(const char *name, unsigned int flags, struct path *path)
}
EXPORT_SYMBOL(kern_path);

/**
* vfs_path_parent_lookup - lookup a parent path relative to a dentry-vfsmount pair
* @filename: filename structure
* @flags: lookup flags
* @parent: pointer to struct path to fill
* @last: last component
* @type: type of the last component
* @root: pointer to struct path of the base directory
*/
int vfs_path_parent_lookup(struct filename *filename, unsigned int flags,
struct path *parent, struct qstr *last, int *type,
const struct path *root)
{
return __filename_parentat(AT_FDCWD, filename, flags, parent, last,
type, root);
}
EXPORT_SYMBOL(vfs_path_parent_lookup);

/**
* vfs_path_lookup - lookup a file path relative to a dentry-vfsmount pair
* @dentry: pointer to dentry of the base directory
Expand Down Expand Up @@ -3852,7 +3882,8 @@ static struct dentry *filename_create(int dfd, struct filename *name,
if (last.name[last.len] && !want_dir)
create_flags = 0;
inode_lock_nested(path->dentry->d_inode, I_MUTEX_PARENT);
dentry = __lookup_hash(&last, path->dentry, reval_flag | create_flags);
dentry = lookup_one_qstr_excl(&last, path->dentry,
reval_flag | create_flags);
if (IS_ERR(dentry))
goto unlock;

Expand Down Expand Up @@ -4214,7 +4245,7 @@ int do_rmdir(int dfd, struct filename *name)
goto exit2;

inode_lock_nested(path.dentry->d_inode, I_MUTEX_PARENT);
dentry = __lookup_hash(&last, path.dentry, lookup_flags);
dentry = lookup_one_qstr_excl(&last, path.dentry, lookup_flags);
error = PTR_ERR(dentry);
if (IS_ERR(dentry))
goto exit3;
Expand Down Expand Up @@ -4348,7 +4379,7 @@ int do_unlinkat(int dfd, struct filename *name)
goto exit2;
retry_deleg:
inode_lock_nested(path.dentry->d_inode, I_MUTEX_PARENT);
dentry = __lookup_hash(&last, path.dentry, lookup_flags);
dentry = lookup_one_qstr_excl(&last, path.dentry, lookup_flags);
error = PTR_ERR(dentry);
if (!IS_ERR(dentry)) {
struct user_namespace *mnt_userns;
Expand Down Expand Up @@ -4922,15 +4953,17 @@ int do_renameat2(int olddfd, struct filename *from, int newdfd,
retry_deleg:
trap = lock_rename(new_path.dentry, old_path.dentry);

old_dentry = __lookup_hash(&old_last, old_path.dentry, lookup_flags);
old_dentry = lookup_one_qstr_excl(&old_last, old_path.dentry,
lookup_flags);
error = PTR_ERR(old_dentry);
if (IS_ERR(old_dentry))
goto exit3;
/* source must exist */
error = -ENOENT;
if (d_is_negative(old_dentry))
goto exit4;
new_dentry = __lookup_hash(&new_last, new_path.dentry, lookup_flags | target_flags);
new_dentry = lookup_one_qstr_excl(&new_last, new_path.dentry,
lookup_flags | target_flags);
error = PTR_ERR(new_dentry);
if (IS_ERR(new_dentry))
goto exit4;
Expand Down

0 comments on commit 6927ffe

Please sign in to comment.