From: NeilBrown <neil@brown.name>
Several callers perform a rename on a dentry they already have, and only
require lookup for the target name. This includes smb/server and a few
different places in overlayfs.
start_renaming_dentry() performs the required lookup and takes the
required lock using lock_rename_child()
It is used in three places in overlayfs and in ksmbd_vfs_rename().
In the ksmbd case, the parent of the source is not important - the
source must be renamed from wherever it is. So start_renaming_dentry()
allows rd->old_parent to be NULL and only checks it if it is non-NULL.
On success rd->old_parent will be the parent of old_dentry with an extra
reference taken. Other start_renaming function also now take the extra
reference and end_renaming() now drops this reference as well.
ovl_lookup_temp(), ovl_parent_lock(), and ovl_parent_unlock() are
all removed as they are no longer needed.
OVL_TEMPNAME_SIZE and ovl_tempname() are now declared in overlayfs.h so
that ovl_check_rename_whiteout() can access them.
ovl_copy_up_workdir() now always cleans up on error.
Reviewed-by: Namjae Jeon <linkinjeon@kernel.org> (for ksmbd part)
Reviewed-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: NeilBrown <neil@brown.name>
---
fs/namei.c | 108 ++++++++++++++++++++++++++++++++++++---
fs/overlayfs/copy_up.c | 54 +++++++++-----------
fs/overlayfs/dir.c | 19 +------
fs/overlayfs/overlayfs.h | 8 +--
fs/overlayfs/super.c | 22 ++++----
fs/overlayfs/util.c | 11 ----
fs/smb/server/vfs.c | 60 ++++------------------
include/linux/namei.h | 2 +
8 files changed, 150 insertions(+), 134 deletions(-)
diff --git a/fs/namei.c b/fs/namei.c
index bad6c9d540f9..4b740048df97 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -3669,7 +3669,7 @@ EXPORT_SYMBOL(unlock_rename);
/**
* __start_renaming - lookup and lock names for rename
- * @rd: rename data containing parent and flags, and
+ * @rd: rename data containing parents and flags, and
* for receiving found dentries
* @lookup_flags: extra flags to pass to ->lookup (e.g. LOOKUP_REVAL,
* LOOKUP_NO_SYMLINKS etc).
@@ -3680,8 +3680,8 @@ EXPORT_SYMBOL(unlock_rename);
* rename.
*
* On success the found dentries are stored in @rd.old_dentry,
- * @rd.new_dentry. These references and the lock are dropped by
- * end_renaming().
+ * @rd.new_dentry and an extra ref is taken on @rd.old_parent.
+ * These references and the lock are dropped by end_renaming().
*
* The passed in qstrs must have the hash calculated, and no permission
* checking is performed.
@@ -3735,6 +3735,7 @@ __start_renaming(struct renamedata *rd, int lookup_flags,
rd->old_dentry = d1;
rd->new_dentry = d2;
+ dget(rd->old_parent);
return 0;
out_dput_d2:
@@ -3748,7 +3749,7 @@ __start_renaming(struct renamedata *rd, int lookup_flags,
/**
* start_renaming - lookup and lock names for rename with permission checking
- * @rd: rename data containing parent and flags, and
+ * @rd: rename data containing parents and flags, and
* for receiving found dentries
* @lookup_flags: extra flags to pass to ->lookup (e.g. LOOKUP_REVAL,
* LOOKUP_NO_SYMLINKS etc).
@@ -3759,8 +3760,8 @@ __start_renaming(struct renamedata *rd, int lookup_flags,
* rename.
*
* On success the found dentries are stored in @rd.old_dentry,
- * @rd.new_dentry. These references and the lock are dropped by
- * end_renaming().
+ * @rd.new_dentry. Also the refcount on @rd->old_parent is increased.
+ * These references and the lock are dropped by end_renaming().
*
* The passed in qstrs need not have the hash calculated, and basic
* eXecute permission checking is performed against @rd.mnt_idmap.
@@ -3782,11 +3783,106 @@ int start_renaming(struct renamedata *rd, int lookup_flags,
}
EXPORT_SYMBOL(start_renaming);
+static int
+__start_renaming_dentry(struct renamedata *rd, int lookup_flags,
+ struct dentry *old_dentry, struct qstr *new_last)
+{
+ struct dentry *trap;
+ struct dentry *d2;
+ int target_flags = LOOKUP_RENAME_TARGET | LOOKUP_CREATE;
+ int err;
+
+ if (rd->flags & RENAME_EXCHANGE)
+ target_flags = 0;
+ if (rd->flags & RENAME_NOREPLACE)
+ target_flags |= LOOKUP_EXCL;
+
+ /* Already have the dentry - need to be sure to lock the correct parent */
+ trap = lock_rename_child(old_dentry, rd->new_parent);
+ if (IS_ERR(trap))
+ return PTR_ERR(trap);
+ if (d_unhashed(old_dentry) ||
+ (rd->old_parent && rd->old_parent != old_dentry->d_parent)) {
+ /* dentry was removed, or moved and explicit parent requested */
+ err = -EINVAL;
+ goto out_unlock;
+ }
+
+ d2 = lookup_one_qstr_excl(new_last, rd->new_parent,
+ lookup_flags | target_flags);
+ err = PTR_ERR(d2);
+ if (IS_ERR(d2))
+ goto out_unlock;
+
+ if (old_dentry == trap) {
+ /* source is an ancestor of target */
+ err = -EINVAL;
+ goto out_dput_d2;
+ }
+
+ if (d2 == trap) {
+ /* target is an ancestor of source */
+ if (rd->flags & RENAME_EXCHANGE)
+ err = -EINVAL;
+ else
+ err = -ENOTEMPTY;
+ goto out_dput_d2;
+ }
+
+ rd->old_dentry = dget(old_dentry);
+ rd->new_dentry = d2;
+ rd->old_parent = dget(old_dentry->d_parent);
+ return 0;
+
+out_dput_d2:
+ dput(d2);
+out_unlock:
+ unlock_rename(old_dentry->d_parent, rd->new_parent);
+ return err;
+}
+
+/**
+ * start_renaming_dentry - lookup and lock name for rename with permission checking
+ * @rd: rename data containing parents and flags, and
+ * for receiving found dentries
+ * @lookup_flags: extra flags to pass to ->lookup (e.g. LOOKUP_REVAL,
+ * LOOKUP_NO_SYMLINKS etc).
+ * @old_dentry: dentry of name to move
+ * @new_last: name of target in @rd.new_parent
+ *
+ * Look up target name and ensure locks are in place for
+ * rename.
+ *
+ * On success the found dentry is stored in @rd.new_dentry and
+ * @rd.old_parent is confirmed to be the parent of @old_dentry. If it
+ * was originally %NULL, it is set. In either case a reference is taken
+ * so that end_renaming() can have a stable reference to unlock.
+ *
+ * References and the lock can be dropped with end_renaming()
+ *
+ * The passed in qstr need not have the hash calculated, and basic
+ * eXecute permission checking is performed against @rd.mnt_idmap.
+ *
+ * Returns: zero or an error.
+ */
+int start_renaming_dentry(struct renamedata *rd, int lookup_flags,
+ struct dentry *old_dentry, struct qstr *new_last)
+{
+ int err;
+
+ err = lookup_one_common(rd->mnt_idmap, new_last, rd->new_parent);
+ if (err)
+ return err;
+ return __start_renaming_dentry(rd, lookup_flags, old_dentry, new_last);
+}
+EXPORT_SYMBOL(start_renaming_dentry);
+
void end_renaming(struct renamedata *rd)
{
unlock_rename(rd->old_parent, rd->new_parent);
dput(rd->old_dentry);
dput(rd->new_dentry);
+ dput(rd->old_parent);
}
EXPORT_SYMBOL(end_renaming);
diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
index e2bdac4317e7..9911a346b477 100644
--- a/fs/overlayfs/copy_up.c
+++ b/fs/overlayfs/copy_up.c
@@ -523,8 +523,8 @@ static int ovl_create_index(struct dentry *dentry, const struct ovl_fh *fh,
{
struct ovl_fs *ofs = OVL_FS(dentry->d_sb);
struct dentry *indexdir = ovl_indexdir(dentry->d_sb);
- struct dentry *index = NULL;
struct dentry *temp = NULL;
+ struct renamedata rd = {};
struct qstr name = { };
int err;
@@ -556,17 +556,15 @@ static int ovl_create_index(struct dentry *dentry, const struct ovl_fh *fh,
if (err)
goto out;
- err = ovl_parent_lock(indexdir, temp);
+ rd.mnt_idmap = ovl_upper_mnt_idmap(ofs);
+ rd.old_parent = indexdir;
+ rd.new_parent = indexdir;
+ err = start_renaming_dentry(&rd, 0, temp, &name);
if (err)
goto out;
- index = ovl_lookup_upper(ofs, name.name, indexdir, name.len);
- if (IS_ERR(index)) {
- err = PTR_ERR(index);
- } else {
- err = ovl_do_rename(ofs, indexdir, temp, indexdir, index, 0);
- dput(index);
- }
- ovl_parent_unlock(indexdir);
+
+ err = ovl_do_rename_rd(&rd);
+ end_renaming(&rd);
out:
if (err)
ovl_cleanup(ofs, indexdir, temp);
@@ -763,7 +761,8 @@ static int ovl_copy_up_workdir(struct ovl_copy_up_ctx *c)
struct ovl_fs *ofs = OVL_FS(c->dentry->d_sb);
struct inode *inode;
struct path path = { .mnt = ovl_upper_mnt(ofs) };
- struct dentry *temp, *upper, *trap;
+ struct renamedata rd = {};
+ struct dentry *temp;
struct ovl_cu_creds cc;
int err;
struct ovl_cattr cattr = {
@@ -807,29 +806,24 @@ static int ovl_copy_up_workdir(struct ovl_copy_up_ctx *c)
* ovl_copy_up_data(), so lock workdir and destdir and make sure that
* temp wasn't moved before copy up completion or cleanup.
*/
- trap = lock_rename(c->workdir, c->destdir);
- if (trap || temp->d_parent != c->workdir) {
- /* temp or workdir moved underneath us? abort without cleanup */
- dput(temp);
+ rd.mnt_idmap = ovl_upper_mnt_idmap(ofs);
+ rd.old_parent = c->workdir;
+ rd.new_parent = c->destdir;
+ rd.flags = 0;
+ err = start_renaming_dentry(&rd, 0, temp,
+ &QSTR_LEN(c->destname.name, c->destname.len));
+ if (err) {
+ /* temp or workdir moved underneath us? map to -EIO */
err = -EIO;
- if (!IS_ERR(trap))
- unlock_rename(c->workdir, c->destdir);
- goto out;
}
-
- err = ovl_copy_up_metadata(c, temp);
if (err)
- goto cleanup;
+ goto cleanup_unlocked;
- upper = ovl_lookup_upper(ofs, c->destname.name, c->destdir,
- c->destname.len);
- err = PTR_ERR(upper);
- if (IS_ERR(upper))
- goto cleanup;
+ err = ovl_copy_up_metadata(c, temp);
+ if (!err)
+ err = ovl_do_rename_rd(&rd);
+ end_renaming(&rd);
- err = ovl_do_rename(ofs, c->workdir, temp, c->destdir, upper, 0);
- unlock_rename(c->workdir, c->destdir);
- dput(upper);
if (err)
goto cleanup_unlocked;
@@ -850,8 +844,6 @@ static int ovl_copy_up_workdir(struct ovl_copy_up_ctx *c)
return err;
-cleanup:
- unlock_rename(c->workdir, c->destdir);
cleanup_unlocked:
ovl_cleanup(ofs, c->workdir, temp);
dput(temp);
diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
index cf6fc48459f3..6b2f88edb497 100644
--- a/fs/overlayfs/dir.c
+++ b/fs/overlayfs/dir.c
@@ -57,8 +57,7 @@ int ovl_cleanup(struct ovl_fs *ofs, struct dentry *workdir,
return 0;
}
-#define OVL_TEMPNAME_SIZE 20
-static void ovl_tempname(char name[OVL_TEMPNAME_SIZE])
+void ovl_tempname(char name[OVL_TEMPNAME_SIZE])
{
static atomic_t temp_id = ATOMIC_INIT(0);
@@ -66,22 +65,6 @@ static void ovl_tempname(char name[OVL_TEMPNAME_SIZE])
snprintf(name, OVL_TEMPNAME_SIZE, "#%x", atomic_inc_return(&temp_id));
}
-struct dentry *ovl_lookup_temp(struct ovl_fs *ofs, struct dentry *workdir)
-{
- struct dentry *temp;
- char name[OVL_TEMPNAME_SIZE];
-
- ovl_tempname(name);
- temp = ovl_lookup_upper(ofs, name, workdir, strlen(name));
- if (!IS_ERR(temp) && temp->d_inode) {
- pr_err("workdir/%s already exists\n", name);
- dput(temp);
- temp = ERR_PTR(-EIO);
- }
-
- return temp;
-}
-
static struct dentry *ovl_start_creating_temp(struct ovl_fs *ofs,
struct dentry *workdir)
{
diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index 3cc85a893b5c..746bc4ad7b37 100644
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -447,11 +447,6 @@ static inline bool ovl_open_flags_need_copy_up(int flags)
}
/* util.c */
-int ovl_parent_lock(struct dentry *parent, struct dentry *child);
-static inline void ovl_parent_unlock(struct dentry *parent)
-{
- inode_unlock(parent->d_inode);
-}
int ovl_get_write_access(struct dentry *dentry);
void ovl_put_write_access(struct dentry *dentry);
void ovl_start_write(struct dentry *dentry);
@@ -888,7 +883,8 @@ struct dentry *ovl_create_real(struct ovl_fs *ofs,
struct dentry *parent, struct dentry *newdentry,
struct ovl_cattr *attr);
int ovl_cleanup(struct ovl_fs *ofs, struct dentry *workdir, struct dentry *dentry);
-struct dentry *ovl_lookup_temp(struct ovl_fs *ofs, struct dentry *workdir);
+#define OVL_TEMPNAME_SIZE 20
+void ovl_tempname(char name[OVL_TEMPNAME_SIZE]);
struct dentry *ovl_create_temp(struct ovl_fs *ofs, struct dentry *workdir,
struct ovl_cattr *attr);
diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index 6e0816c1147a..a721ef2b90e8 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -566,9 +566,10 @@ static int ovl_check_rename_whiteout(struct ovl_fs *ofs)
{
struct dentry *workdir = ofs->workdir;
struct dentry *temp;
- struct dentry *dest;
struct dentry *whiteout;
struct name_snapshot name;
+ struct renamedata rd = {};
+ char name2[OVL_TEMPNAME_SIZE];
int err;
temp = ovl_create_temp(ofs, workdir, OVL_CATTR(S_IFREG | 0));
@@ -576,23 +577,21 @@ static int ovl_check_rename_whiteout(struct ovl_fs *ofs)
if (IS_ERR(temp))
return err;
- err = ovl_parent_lock(workdir, temp);
+ rd.mnt_idmap = ovl_upper_mnt_idmap(ofs);
+ rd.old_parent = workdir;
+ rd.new_parent = workdir;
+ rd.flags = RENAME_WHITEOUT;
+ ovl_tempname(name2);
+ err = start_renaming_dentry(&rd, 0, temp, &QSTR(name2));
if (err) {
dput(temp);
return err;
}
- dest = ovl_lookup_temp(ofs, workdir);
- err = PTR_ERR(dest);
- if (IS_ERR(dest)) {
- dput(temp);
- ovl_parent_unlock(workdir);
- return err;
- }
/* Name is inline and stable - using snapshot as a copy helper */
take_dentry_name_snapshot(&name, temp);
- err = ovl_do_rename(ofs, workdir, temp, workdir, dest, RENAME_WHITEOUT);
- ovl_parent_unlock(workdir);
+ err = ovl_do_rename_rd(&rd);
+ end_renaming(&rd);
if (err) {
if (err == -EINVAL)
err = 0;
@@ -616,7 +615,6 @@ static int ovl_check_rename_whiteout(struct ovl_fs *ofs)
ovl_cleanup(ofs, workdir, temp);
release_dentry_name_snapshot(&name);
dput(temp);
- dput(dest);
return err;
}
diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
index 2da1c035f716..fffc22859211 100644
--- a/fs/overlayfs/util.c
+++ b/fs/overlayfs/util.c
@@ -1548,14 +1548,3 @@ void ovl_copyattr(struct inode *inode)
i_size_write(inode, i_size_read(realinode));
spin_unlock(&inode->i_lock);
}
-
-int ovl_parent_lock(struct dentry *parent, struct dentry *child)
-{
- inode_lock_nested(parent->d_inode, I_MUTEX_PARENT);
- if (!child ||
- (!d_unhashed(child) && child->d_parent == parent))
- return 0;
-
- inode_unlock(parent->d_inode);
- return -EINVAL;
-}
diff --git a/fs/smb/server/vfs.c b/fs/smb/server/vfs.c
index 7c4ddc43ab39..f54b5b0aaba2 100644
--- a/fs/smb/server/vfs.c
+++ b/fs/smb/server/vfs.c
@@ -663,7 +663,6 @@ int ksmbd_vfs_link(struct ksmbd_work *work, const char *oldname,
int ksmbd_vfs_rename(struct ksmbd_work *work, const struct path *old_path,
char *newname, int flags)
{
- struct dentry *old_parent, *new_dentry, *trap;
struct dentry *old_child = old_path->dentry;
struct path new_path;
struct qstr new_last;
@@ -673,7 +672,6 @@ int ksmbd_vfs_rename(struct ksmbd_work *work, const struct path *old_path,
struct ksmbd_file *parent_fp;
int new_type;
int err, lookup_flags = LOOKUP_NO_SYMLINKS;
- int target_lookup_flags = LOOKUP_RENAME_TARGET | LOOKUP_CREATE;
if (ksmbd_override_fsids(work))
return -ENOMEM;
@@ -684,14 +682,6 @@ int ksmbd_vfs_rename(struct ksmbd_work *work, const struct path *old_path,
goto revert_fsids;
}
- /*
- * explicitly handle file overwrite case, for compatibility with
- * filesystems that may not support rename flags (e.g: fuse)
- */
- if (flags & RENAME_NOREPLACE)
- target_lookup_flags |= LOOKUP_EXCL;
- flags &= ~(RENAME_NOREPLACE);
-
retry:
err = vfs_path_parent_lookup(to, lookup_flags | LOOKUP_BENEATH,
&new_path, &new_last, &new_type,
@@ -708,17 +698,14 @@ int ksmbd_vfs_rename(struct ksmbd_work *work, const struct path *old_path,
if (err)
goto out2;
- trap = lock_rename_child(old_child, new_path.dentry);
- if (IS_ERR(trap)) {
- err = PTR_ERR(trap);
+ rd.mnt_idmap = mnt_idmap(old_path->mnt);
+ rd.old_parent = NULL;
+ rd.new_parent = new_path.dentry;
+ rd.flags = flags;
+ rd.delegated_inode = NULL,
+ err = start_renaming_dentry(&rd, lookup_flags, old_child, &new_last);
+ if (err)
goto out_drop_write;
- }
-
- old_parent = dget(old_child->d_parent);
- if (d_unhashed(old_child)) {
- err = -EINVAL;
- goto out3;
- }
parent_fp = ksmbd_lookup_fd_inode(old_child->d_parent);
if (parent_fp) {
@@ -731,44 +718,17 @@ int ksmbd_vfs_rename(struct ksmbd_work *work, const struct path *old_path,
ksmbd_fd_put(work, parent_fp);
}
- new_dentry = lookup_one_qstr_excl(&new_last, new_path.dentry,
- lookup_flags | target_lookup_flags);
- if (IS_ERR(new_dentry)) {
- err = PTR_ERR(new_dentry);
- goto out3;
- }
-
- if (d_is_symlink(new_dentry)) {
+ if (d_is_symlink(rd.new_dentry)) {
err = -EACCES;
- goto out4;
- }
-
- if (old_child == trap) {
- err = -EINVAL;
- goto out4;
- }
-
- if (new_dentry == trap) {
- err = -ENOTEMPTY;
- goto out4;
+ goto out3;
}
- rd.mnt_idmap = mnt_idmap(old_path->mnt),
- rd.old_parent = old_parent,
- rd.old_dentry = old_child,
- rd.new_parent = new_path.dentry,
- rd.new_dentry = new_dentry,
- rd.flags = flags,
- rd.delegated_inode = NULL,
err = vfs_rename(&rd);
if (err)
ksmbd_debug(VFS, "vfs_rename failed err %d\n", err);
-out4:
- dput(new_dentry);
out3:
- dput(old_parent);
- unlock_rename(old_parent, new_path.dentry);
+ end_renaming(&rd);
out_drop_write:
mnt_drop_write(old_path->mnt);
out2:
diff --git a/include/linux/namei.h b/include/linux/namei.h
index 7fdd9fdcbd2b..c47713e9867c 100644
--- a/include/linux/namei.h
+++ b/include/linux/namei.h
@@ -159,6 +159,8 @@ extern struct dentry *lock_rename_child(struct dentry *, struct dentry *);
extern void unlock_rename(struct dentry *, struct dentry *);
int start_renaming(struct renamedata *rd, int lookup_flags,
struct qstr *old_last, struct qstr *new_last);
+int start_renaming_dentry(struct renamedata *rd, int lookup_flags,
+ struct dentry *old_dentry, struct qstr *new_last);
void end_renaming(struct renamedata *rd);
/**
--
2.50.0.107.gf914562f5916.dirtyOn Thu, 2025-11-06 at 11:50 +1100, NeilBrown wrote:
> From: NeilBrown <neil@brown.name>
>
> Several callers perform a rename on a dentry they already have, and only
> require lookup for the target name. This includes smb/server and a few
> different places in overlayfs.
>
> start_renaming_dentry() performs the required lookup and takes the
> required lock using lock_rename_child()
>
> It is used in three places in overlayfs and in ksmbd_vfs_rename().
>
> In the ksmbd case, the parent of the source is not important - the
> source must be renamed from wherever it is. So start_renaming_dentry()
> allows rd->old_parent to be NULL and only checks it if it is non-NULL.
> On success rd->old_parent will be the parent of old_dentry with an extra
> reference taken. Other start_renaming function also now take the extra
> reference and end_renaming() now drops this reference as well.
>
> ovl_lookup_temp(), ovl_parent_lock(), and ovl_parent_unlock() are
> all removed as they are no longer needed.
>
> OVL_TEMPNAME_SIZE and ovl_tempname() are now declared in overlayfs.h so
> that ovl_check_rename_whiteout() can access them.
>
> ovl_copy_up_workdir() now always cleans up on error.
>
> Reviewed-by: Namjae Jeon <linkinjeon@kernel.org> (for ksmbd part)
> Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> Signed-off-by: NeilBrown <neil@brown.name>
> ---
> fs/namei.c | 108 ++++++++++++++++++++++++++++++++++++---
> fs/overlayfs/copy_up.c | 54 +++++++++-----------
> fs/overlayfs/dir.c | 19 +------
> fs/overlayfs/overlayfs.h | 8 +--
> fs/overlayfs/super.c | 22 ++++----
> fs/overlayfs/util.c | 11 ----
> fs/smb/server/vfs.c | 60 ++++------------------
> include/linux/namei.h | 2 +
> 8 files changed, 150 insertions(+), 134 deletions(-)
>
> diff --git a/fs/namei.c b/fs/namei.c
> index bad6c9d540f9..4b740048df97 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -3669,7 +3669,7 @@ EXPORT_SYMBOL(unlock_rename);
>
> /**
> * __start_renaming - lookup and lock names for rename
> - * @rd: rename data containing parent and flags, and
> + * @rd: rename data containing parents and flags, and
> * for receiving found dentries
> * @lookup_flags: extra flags to pass to ->lookup (e.g. LOOKUP_REVAL,
> * LOOKUP_NO_SYMLINKS etc).
> @@ -3680,8 +3680,8 @@ EXPORT_SYMBOL(unlock_rename);
> * rename.
> *
> * On success the found dentries are stored in @rd.old_dentry,
> - * @rd.new_dentry. These references and the lock are dropped by
> - * end_renaming().
> + * @rd.new_dentry and an extra ref is taken on @rd.old_parent.
> + * These references and the lock are dropped by end_renaming().
> *
> * The passed in qstrs must have the hash calculated, and no permission
> * checking is performed.
> @@ -3735,6 +3735,7 @@ __start_renaming(struct renamedata *rd, int lookup_flags,
>
> rd->old_dentry = d1;
> rd->new_dentry = d2;
> + dget(rd->old_parent);
> return 0;
>
> out_dput_d2:
> @@ -3748,7 +3749,7 @@ __start_renaming(struct renamedata *rd, int lookup_flags,
>
> /**
> * start_renaming - lookup and lock names for rename with permission checking
> - * @rd: rename data containing parent and flags, and
> + * @rd: rename data containing parents and flags, and
> * for receiving found dentries
> * @lookup_flags: extra flags to pass to ->lookup (e.g. LOOKUP_REVAL,
> * LOOKUP_NO_SYMLINKS etc).
> @@ -3759,8 +3760,8 @@ __start_renaming(struct renamedata *rd, int lookup_flags,
> * rename.
> *
> * On success the found dentries are stored in @rd.old_dentry,
> - * @rd.new_dentry. These references and the lock are dropped by
> - * end_renaming().
> + * @rd.new_dentry. Also the refcount on @rd->old_parent is increased.
> + * These references and the lock are dropped by end_renaming().
> *
> * The passed in qstrs need not have the hash calculated, and basic
> * eXecute permission checking is performed against @rd.mnt_idmap.
> @@ -3782,11 +3783,106 @@ int start_renaming(struct renamedata *rd, int lookup_flags,
> }
> EXPORT_SYMBOL(start_renaming);
>
> +static int
> +__start_renaming_dentry(struct renamedata *rd, int lookup_flags,
> + struct dentry *old_dentry, struct qstr *new_last)
> +{
> + struct dentry *trap;
> + struct dentry *d2;
> + int target_flags = LOOKUP_RENAME_TARGET | LOOKUP_CREATE;
> + int err;
> +
> + if (rd->flags & RENAME_EXCHANGE)
> + target_flags = 0;
> + if (rd->flags & RENAME_NOREPLACE)
> + target_flags |= LOOKUP_EXCL;
> +
> + /* Already have the dentry - need to be sure to lock the correct parent */
> + trap = lock_rename_child(old_dentry, rd->new_parent);
> + if (IS_ERR(trap))
> + return PTR_ERR(trap);
> + if (d_unhashed(old_dentry) ||
> + (rd->old_parent && rd->old_parent != old_dentry->d_parent)) {
> + /* dentry was removed, or moved and explicit parent requested */
> + err = -EINVAL;
> + goto out_unlock;
> + }
> +
> + d2 = lookup_one_qstr_excl(new_last, rd->new_parent,
> + lookup_flags | target_flags);
> + err = PTR_ERR(d2);
> + if (IS_ERR(d2))
> + goto out_unlock;
> +
> + if (old_dentry == trap) {
> + /* source is an ancestor of target */
> + err = -EINVAL;
> + goto out_dput_d2;
> + }
> +
> + if (d2 == trap) {
> + /* target is an ancestor of source */
> + if (rd->flags & RENAME_EXCHANGE)
> + err = -EINVAL;
> + else
> + err = -ENOTEMPTY;
> + goto out_dput_d2;
> + }
> +
> + rd->old_dentry = dget(old_dentry);
> + rd->new_dentry = d2;
> + rd->old_parent = dget(old_dentry->d_parent);
> + return 0;
> +
> +out_dput_d2:
> + dput(d2);
> +out_unlock:
> + unlock_rename(old_dentry->d_parent, rd->new_parent);
> + return err;
> +}
> +
> +/**
> + * start_renaming_dentry - lookup and lock name for rename with permission checking
> + * @rd: rename data containing parents and flags, and
> + * for receiving found dentries
> + * @lookup_flags: extra flags to pass to ->lookup (e.g. LOOKUP_REVAL,
> + * LOOKUP_NO_SYMLINKS etc).
> + * @old_dentry: dentry of name to move
> + * @new_last: name of target in @rd.new_parent
> + *
> + * Look up target name and ensure locks are in place for
> + * rename.
> + *
> + * On success the found dentry is stored in @rd.new_dentry and
> + * @rd.old_parent is confirmed to be the parent of @old_dentry. If it
> + * was originally %NULL, it is set. In either case a reference is taken
> + * so that end_renaming() can have a stable reference to unlock.
> + *
> + * References and the lock can be dropped with end_renaming()
> + *
> + * The passed in qstr need not have the hash calculated, and basic
> + * eXecute permission checking is performed against @rd.mnt_idmap.
> + *
> + * Returns: zero or an error.
> + */
> +int start_renaming_dentry(struct renamedata *rd, int lookup_flags,
> + struct dentry *old_dentry, struct qstr *new_last)
> +{
> + int err;
> +
> + err = lookup_one_common(rd->mnt_idmap, new_last, rd->new_parent);
> + if (err)
> + return err;
> + return __start_renaming_dentry(rd, lookup_flags, old_dentry, new_last);
> +}
> +EXPORT_SYMBOL(start_renaming_dentry);
> +
> void end_renaming(struct renamedata *rd)
> {
> unlock_rename(rd->old_parent, rd->new_parent);
> dput(rd->old_dentry);
> dput(rd->new_dentry);
> + dput(rd->old_parent);
> }
> EXPORT_SYMBOL(end_renaming);
>
> diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
> index e2bdac4317e7..9911a346b477 100644
> --- a/fs/overlayfs/copy_up.c
> +++ b/fs/overlayfs/copy_up.c
> @@ -523,8 +523,8 @@ static int ovl_create_index(struct dentry *dentry, const struct ovl_fh *fh,
> {
> struct ovl_fs *ofs = OVL_FS(dentry->d_sb);
> struct dentry *indexdir = ovl_indexdir(dentry->d_sb);
> - struct dentry *index = NULL;
> struct dentry *temp = NULL;
> + struct renamedata rd = {};
> struct qstr name = { };
> int err;
>
> @@ -556,17 +556,15 @@ static int ovl_create_index(struct dentry *dentry, const struct ovl_fh *fh,
> if (err)
> goto out;
>
> - err = ovl_parent_lock(indexdir, temp);
> + rd.mnt_idmap = ovl_upper_mnt_idmap(ofs);
> + rd.old_parent = indexdir;
> + rd.new_parent = indexdir;
> + err = start_renaming_dentry(&rd, 0, temp, &name);
> if (err)
> goto out;
> - index = ovl_lookup_upper(ofs, name.name, indexdir, name.len);
> - if (IS_ERR(index)) {
> - err = PTR_ERR(index);
> - } else {
> - err = ovl_do_rename(ofs, indexdir, temp, indexdir, index, 0);
> - dput(index);
> - }
> - ovl_parent_unlock(indexdir);
> +
> + err = ovl_do_rename_rd(&rd);
> + end_renaming(&rd);
> out:
> if (err)
> ovl_cleanup(ofs, indexdir, temp);
> @@ -763,7 +761,8 @@ static int ovl_copy_up_workdir(struct ovl_copy_up_ctx *c)
> struct ovl_fs *ofs = OVL_FS(c->dentry->d_sb);
> struct inode *inode;
> struct path path = { .mnt = ovl_upper_mnt(ofs) };
> - struct dentry *temp, *upper, *trap;
> + struct renamedata rd = {};
> + struct dentry *temp;
> struct ovl_cu_creds cc;
> int err;
> struct ovl_cattr cattr = {
> @@ -807,29 +806,24 @@ static int ovl_copy_up_workdir(struct ovl_copy_up_ctx *c)
> * ovl_copy_up_data(), so lock workdir and destdir and make sure that
> * temp wasn't moved before copy up completion or cleanup.
> */
> - trap = lock_rename(c->workdir, c->destdir);
> - if (trap || temp->d_parent != c->workdir) {
> - /* temp or workdir moved underneath us? abort without cleanup */
> - dput(temp);
> + rd.mnt_idmap = ovl_upper_mnt_idmap(ofs);
> + rd.old_parent = c->workdir;
> + rd.new_parent = c->destdir;
> + rd.flags = 0;
> + err = start_renaming_dentry(&rd, 0, temp,
> + &QSTR_LEN(c->destname.name, c->destname.len));
> + if (err) {
> + /* temp or workdir moved underneath us? map to -EIO */
> err = -EIO;
> - if (!IS_ERR(trap))
> - unlock_rename(c->workdir, c->destdir);
> - goto out;
> }
> -
> - err = ovl_copy_up_metadata(c, temp);
> if (err)
> - goto cleanup;
> + goto cleanup_unlocked;
>
> - upper = ovl_lookup_upper(ofs, c->destname.name, c->destdir,
> - c->destname.len);
> - err = PTR_ERR(upper);
> - if (IS_ERR(upper))
> - goto cleanup;
> + err = ovl_copy_up_metadata(c, temp);
> + if (!err)
> + err = ovl_do_rename_rd(&rd);
> + end_renaming(&rd);
>
> - err = ovl_do_rename(ofs, c->workdir, temp, c->destdir, upper, 0);
> - unlock_rename(c->workdir, c->destdir);
> - dput(upper);
> if (err)
> goto cleanup_unlocked;
>
> @@ -850,8 +844,6 @@ static int ovl_copy_up_workdir(struct ovl_copy_up_ctx *c)
>
> return err;
>
> -cleanup:
> - unlock_rename(c->workdir, c->destdir);
> cleanup_unlocked:
> ovl_cleanup(ofs, c->workdir, temp);
> dput(temp);
> diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
> index cf6fc48459f3..6b2f88edb497 100644
> --- a/fs/overlayfs/dir.c
> +++ b/fs/overlayfs/dir.c
> @@ -57,8 +57,7 @@ int ovl_cleanup(struct ovl_fs *ofs, struct dentry *workdir,
> return 0;
> }
>
> -#define OVL_TEMPNAME_SIZE 20
> -static void ovl_tempname(char name[OVL_TEMPNAME_SIZE])
> +void ovl_tempname(char name[OVL_TEMPNAME_SIZE])
> {
> static atomic_t temp_id = ATOMIC_INIT(0);
>
> @@ -66,22 +65,6 @@ static void ovl_tempname(char name[OVL_TEMPNAME_SIZE])
> snprintf(name, OVL_TEMPNAME_SIZE, "#%x", atomic_inc_return(&temp_id));
> }
>
> -struct dentry *ovl_lookup_temp(struct ovl_fs *ofs, struct dentry *workdir)
> -{
> - struct dentry *temp;
> - char name[OVL_TEMPNAME_SIZE];
> -
> - ovl_tempname(name);
> - temp = ovl_lookup_upper(ofs, name, workdir, strlen(name));
> - if (!IS_ERR(temp) && temp->d_inode) {
> - pr_err("workdir/%s already exists\n", name);
> - dput(temp);
> - temp = ERR_PTR(-EIO);
> - }
> -
> - return temp;
> -}
> -
> static struct dentry *ovl_start_creating_temp(struct ovl_fs *ofs,
> struct dentry *workdir)
> {
> diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
> index 3cc85a893b5c..746bc4ad7b37 100644
> --- a/fs/overlayfs/overlayfs.h
> +++ b/fs/overlayfs/overlayfs.h
> @@ -447,11 +447,6 @@ static inline bool ovl_open_flags_need_copy_up(int flags)
> }
>
> /* util.c */
> -int ovl_parent_lock(struct dentry *parent, struct dentry *child);
> -static inline void ovl_parent_unlock(struct dentry *parent)
> -{
> - inode_unlock(parent->d_inode);
> -}
> int ovl_get_write_access(struct dentry *dentry);
> void ovl_put_write_access(struct dentry *dentry);
> void ovl_start_write(struct dentry *dentry);
> @@ -888,7 +883,8 @@ struct dentry *ovl_create_real(struct ovl_fs *ofs,
> struct dentry *parent, struct dentry *newdentry,
> struct ovl_cattr *attr);
> int ovl_cleanup(struct ovl_fs *ofs, struct dentry *workdir, struct dentry *dentry);
> -struct dentry *ovl_lookup_temp(struct ovl_fs *ofs, struct dentry *workdir);
> +#define OVL_TEMPNAME_SIZE 20
> +void ovl_tempname(char name[OVL_TEMPNAME_SIZE]);
> struct dentry *ovl_create_temp(struct ovl_fs *ofs, struct dentry *workdir,
> struct ovl_cattr *attr);
>
> diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
> index 6e0816c1147a..a721ef2b90e8 100644
> --- a/fs/overlayfs/super.c
> +++ b/fs/overlayfs/super.c
> @@ -566,9 +566,10 @@ static int ovl_check_rename_whiteout(struct ovl_fs *ofs)
> {
> struct dentry *workdir = ofs->workdir;
> struct dentry *temp;
> - struct dentry *dest;
> struct dentry *whiteout;
> struct name_snapshot name;
> + struct renamedata rd = {};
> + char name2[OVL_TEMPNAME_SIZE];
> int err;
>
> temp = ovl_create_temp(ofs, workdir, OVL_CATTR(S_IFREG | 0));
> @@ -576,23 +577,21 @@ static int ovl_check_rename_whiteout(struct ovl_fs *ofs)
> if (IS_ERR(temp))
> return err;
>
> - err = ovl_parent_lock(workdir, temp);
> + rd.mnt_idmap = ovl_upper_mnt_idmap(ofs);
> + rd.old_parent = workdir;
> + rd.new_parent = workdir;
> + rd.flags = RENAME_WHITEOUT;
> + ovl_tempname(name2);
> + err = start_renaming_dentry(&rd, 0, temp, &QSTR(name2));
> if (err) {
> dput(temp);
> return err;
> }
> - dest = ovl_lookup_temp(ofs, workdir);
> - err = PTR_ERR(dest);
> - if (IS_ERR(dest)) {
> - dput(temp);
> - ovl_parent_unlock(workdir);
> - return err;
> - }
>
> /* Name is inline and stable - using snapshot as a copy helper */
> take_dentry_name_snapshot(&name, temp);
> - err = ovl_do_rename(ofs, workdir, temp, workdir, dest, RENAME_WHITEOUT);
> - ovl_parent_unlock(workdir);
> + err = ovl_do_rename_rd(&rd);
> + end_renaming(&rd);
> if (err) {
> if (err == -EINVAL)
> err = 0;
> @@ -616,7 +615,6 @@ static int ovl_check_rename_whiteout(struct ovl_fs *ofs)
> ovl_cleanup(ofs, workdir, temp);
> release_dentry_name_snapshot(&name);
> dput(temp);
> - dput(dest);
>
> return err;
> }
> diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
> index 2da1c035f716..fffc22859211 100644
> --- a/fs/overlayfs/util.c
> +++ b/fs/overlayfs/util.c
> @@ -1548,14 +1548,3 @@ void ovl_copyattr(struct inode *inode)
> i_size_write(inode, i_size_read(realinode));
> spin_unlock(&inode->i_lock);
> }
> -
> -int ovl_parent_lock(struct dentry *parent, struct dentry *child)
> -{
> - inode_lock_nested(parent->d_inode, I_MUTEX_PARENT);
> - if (!child ||
> - (!d_unhashed(child) && child->d_parent == parent))
> - return 0;
> -
> - inode_unlock(parent->d_inode);
> - return -EINVAL;
> -}
> diff --git a/fs/smb/server/vfs.c b/fs/smb/server/vfs.c
> index 7c4ddc43ab39..f54b5b0aaba2 100644
> --- a/fs/smb/server/vfs.c
> +++ b/fs/smb/server/vfs.c
> @@ -663,7 +663,6 @@ int ksmbd_vfs_link(struct ksmbd_work *work, const char *oldname,
> int ksmbd_vfs_rename(struct ksmbd_work *work, const struct path *old_path,
> char *newname, int flags)
> {
> - struct dentry *old_parent, *new_dentry, *trap;
> struct dentry *old_child = old_path->dentry;
> struct path new_path;
> struct qstr new_last;
> @@ -673,7 +672,6 @@ int ksmbd_vfs_rename(struct ksmbd_work *work, const struct path *old_path,
> struct ksmbd_file *parent_fp;
> int new_type;
> int err, lookup_flags = LOOKUP_NO_SYMLINKS;
> - int target_lookup_flags = LOOKUP_RENAME_TARGET | LOOKUP_CREATE;
>
> if (ksmbd_override_fsids(work))
> return -ENOMEM;
> @@ -684,14 +682,6 @@ int ksmbd_vfs_rename(struct ksmbd_work *work, const struct path *old_path,
> goto revert_fsids;
> }
>
> - /*
> - * explicitly handle file overwrite case, for compatibility with
> - * filesystems that may not support rename flags (e.g: fuse)
> - */
> - if (flags & RENAME_NOREPLACE)
> - target_lookup_flags |= LOOKUP_EXCL;
> - flags &= ~(RENAME_NOREPLACE);
> -
> retry:
> err = vfs_path_parent_lookup(to, lookup_flags | LOOKUP_BENEATH,
> &new_path, &new_last, &new_type,
> @@ -708,17 +698,14 @@ int ksmbd_vfs_rename(struct ksmbd_work *work, const struct path *old_path,
> if (err)
> goto out2;
>
> - trap = lock_rename_child(old_child, new_path.dentry);
> - if (IS_ERR(trap)) {
> - err = PTR_ERR(trap);
> + rd.mnt_idmap = mnt_idmap(old_path->mnt);
> + rd.old_parent = NULL;
> + rd.new_parent = new_path.dentry;
> + rd.flags = flags;
> + rd.delegated_inode = NULL,
> + err = start_renaming_dentry(&rd, lookup_flags, old_child, &new_last);
> + if (err)
> goto out_drop_write;
> - }
> -
> - old_parent = dget(old_child->d_parent);
> - if (d_unhashed(old_child)) {
> - err = -EINVAL;
> - goto out3;
> - }
>
> parent_fp = ksmbd_lookup_fd_inode(old_child->d_parent);
> if (parent_fp) {
> @@ -731,44 +718,17 @@ int ksmbd_vfs_rename(struct ksmbd_work *work, const struct path *old_path,
> ksmbd_fd_put(work, parent_fp);
> }
>
> - new_dentry = lookup_one_qstr_excl(&new_last, new_path.dentry,
> - lookup_flags | target_lookup_flags);
> - if (IS_ERR(new_dentry)) {
> - err = PTR_ERR(new_dentry);
> - goto out3;
> - }
> -
> - if (d_is_symlink(new_dentry)) {
> + if (d_is_symlink(rd.new_dentry)) {
> err = -EACCES;
> - goto out4;
> - }
> -
> - if (old_child == trap) {
> - err = -EINVAL;
> - goto out4;
> - }
> -
> - if (new_dentry == trap) {
> - err = -ENOTEMPTY;
> - goto out4;
> + goto out3;
> }
>
> - rd.mnt_idmap = mnt_idmap(old_path->mnt),
> - rd.old_parent = old_parent,
> - rd.old_dentry = old_child,
> - rd.new_parent = new_path.dentry,
> - rd.new_dentry = new_dentry,
> - rd.flags = flags,
> - rd.delegated_inode = NULL,
> err = vfs_rename(&rd);
> if (err)
> ksmbd_debug(VFS, "vfs_rename failed err %d\n", err);
>
> -out4:
> - dput(new_dentry);
> out3:
> - dput(old_parent);
> - unlock_rename(old_parent, new_path.dentry);
> + end_renaming(&rd);
> out_drop_write:
> mnt_drop_write(old_path->mnt);
> out2:
> diff --git a/include/linux/namei.h b/include/linux/namei.h
> index 7fdd9fdcbd2b..c47713e9867c 100644
> --- a/include/linux/namei.h
> +++ b/include/linux/namei.h
> @@ -159,6 +159,8 @@ extern struct dentry *lock_rename_child(struct dentry *, struct dentry *);
> extern void unlock_rename(struct dentry *, struct dentry *);
> int start_renaming(struct renamedata *rd, int lookup_flags,
> struct qstr *old_last, struct qstr *new_last);
> +int start_renaming_dentry(struct renamedata *rd, int lookup_flags,
> + struct dentry *old_dentry, struct qstr *new_last);
> void end_renaming(struct renamedata *rd);
>
> /**
Reviewed-by: Jeff Layton <jlayton@kernel.org>
© 2016 - 2025 Red Hat, Inc.