Unverified Commit ac50950c authored by NeilBrown's avatar NeilBrown Committed by Christian Brauner
Browse files

VFS/ovl/smb: introduce start_renaming_dentry() 



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: default avatarNamjae Jeon <linkinjeon@kernel.org>
Reviewed-by: default avatarAmir Goldstein <amir73il@gmail.com>
Reviewed-by: default avatarJeff Layton <jlayton@kernel.org>
Signed-off-by: default avatarNeilBrown <neil@brown.name>
Link: https://patch.msgid.link/20251113002050.676694-12-neilb@ownmail.net


Tested-by: default avatar <syzbot@syzkaller.appspotmail.com>
Signed-off-by: default avatarChristian Brauner <brauner@kernel.org>
parent 5c875272

fs/namei.c

+102 −6
Original line number Diff line number Diff line
@@ -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);

fs/overlayfs/copy_up.c

+23 −31
Original line number Diff line number Diff line
@@ -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);

fs/overlayfs/dir.c

+1 −18
Original line number Diff line number Diff line
@@ -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)

{

fs/overlayfs/overlayfs.h

+2 −6
Original line number Diff line number Diff line
@@ -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);

fs/overlayfs/super.c

+10 −12
Original line number Diff line number Diff line
@@ -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;

}