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

Add start_renaming_two_dentries() 



A few callers want to lock for a rename and already have both dentries.
Also debugfs does want to perform a lookup but doesn't want permission
checking, so start_renaming_dentry() cannot be used.

This patch introduces start_renaming_two_dentries() which is given both
dentries.  debugfs performs one lookup itself.  As it will only continue
with a negative dentry and as those cannot be renamed or unlinked, it is
safe to do the lookup before getting the rename locks.

overlayfs uses start_renaming_two_dentries() in three places and  selinux
uses it twice in sel_make_policy_nodes().

In sel_make_policy_nodes() we now lock for rename twice instead of just
once so the combined operation is no longer atomic w.r.t the parent
directory locks.  As selinux_state.policy_mutex is held across the whole
operation this does not open up any interesting races.

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-13-neilb@ownmail.net


Signed-off-by: default avatarChristian Brauner <brauner@kernel.org>
parent ac50950c

fs/debugfs/inode.c

+22 −26
Original line number Diff line number Diff line
@@ -842,7 +842,8 @@ int __printf(2, 3) debugfs_change_name(struct dentry *dentry, const char *fmt, . 
	int error = 0;

	const char *new_name;

	struct name_snapshot old_name;

	struct dentry *parent, *target;

	struct dentry *target;

	struct renamedata rd = {};

	struct inode *dir;

	va_list ap;
@@ -855,36 +856,31 @@ int __printf(2, 3) debugfs_change_name(struct dentry *dentry, const char *fmt, . 
	if (!new_name)

		return -ENOMEM;



	parent = dget_parent(dentry);

	dir = d_inode(parent);

	inode_lock(dir);

	rd.old_parent = dget_parent(dentry);

	rd.new_parent = rd.old_parent;

	rd.flags = RENAME_NOREPLACE;

	target = lookup_noperm_unlocked(&QSTR(new_name), rd.new_parent);

	if (IS_ERR(target))

		return PTR_ERR(target);



	take_dentry_name_snapshot(&old_name, dentry);



	if (WARN_ON_ONCE(dentry->d_parent != parent)) {

		error = -EINVAL;

		goto out;

	}

	if (strcmp(old_name.name.name, new_name) == 0)

		goto out;

	target = lookup_noperm(&QSTR(new_name), parent);

	if (IS_ERR(target)) {

		error = PTR_ERR(target);

		goto out;

	}

	if (d_really_is_positive(target)) {

		dput(target);

		error = -EINVAL;

	error = start_renaming_two_dentries(&rd, dentry, target);

	if (error) {

		if (error == -EEXIST && target == dentry)

			/* it isn't an error to rename a thing to itself */

			error = 0;

		goto out;

	}

	simple_rename_timestamp(dir, dentry, dir, target);

	d_move(dentry, target);

	dput(target);



	dir = d_inode(rd.old_parent);

	take_dentry_name_snapshot(&old_name, dentry);

	simple_rename_timestamp(dir, dentry, dir, rd.new_dentry);

	d_move(dentry, rd.new_dentry);

	fsnotify_move(dir, dir, &old_name.name, d_is_dir(dentry), NULL, dentry);

out:

	release_dentry_name_snapshot(&old_name);

	inode_unlock(dir);

	dput(parent);

	end_renaming(&rd);

out:

	dput(rd.old_parent);

	dput(target);

	kfree_const(new_name);

	return error;

}

fs/namei.c

+65 −0
Original line number Diff line number Diff line
@@ -3877,6 +3877,71 @@ int start_renaming_dentry(struct renamedata *rd, int lookup_flags, 
}

EXPORT_SYMBOL(start_renaming_dentry);



/**

 * start_renaming_two_dentries - Lock to dentries in given parents for rename

 * @rd:           rename data containing parent

 * @old_dentry:   dentry of name to move

 * @new_dentry:   dentry to move to

 *

 * Ensure locks are in place for rename and check parentage is still correct.

 *

 * On success the two dentries are stored in @rd.old_dentry and

 * @rd.new_dentry and @rd.old_parent and @rd.new_parent are confirmed to

 * be the parents of the dentries.

 *

 * References and the lock can be dropped with end_renaming()

 *

 * Returns: zero or an error.

 */

int

start_renaming_two_dentries(struct renamedata *rd,

			    struct dentry *old_dentry, struct dentry *new_dentry)

{

	struct dentry *trap;

	int err;



	/* 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);

	err = -EINVAL;

	if (d_unhashed(old_dentry) ||

	    (rd->old_parent && rd->old_parent != old_dentry->d_parent))

		/* old_dentry was removed, or moved and explicit parent requested */

		goto out_unlock;

	if (d_unhashed(new_dentry) ||

	    rd->new_parent != new_dentry->d_parent)

		/* new_dentry was removed or moved */

		goto out_unlock;



	if (old_dentry == trap)

		/* source is an ancestor of target */

		goto out_unlock;



	if (new_dentry == trap) {

		/* target is an ancestor of source */

		if (rd->flags & RENAME_EXCHANGE)

			err = -EINVAL;

		else

			err = -ENOTEMPTY;

		goto out_unlock;

	}



	err = -EEXIST;

	if (d_is_positive(new_dentry) && (rd->flags & RENAME_NOREPLACE))

		goto out_unlock;



	rd->old_dentry = dget(old_dentry);

	rd->new_dentry = dget(new_dentry);

	rd->old_parent = dget(old_dentry->d_parent);

	return 0;



out_unlock:

	unlock_rename(old_dentry->d_parent, rd->new_parent);

	return err;

}

EXPORT_SYMBOL(start_renaming_two_dentries);



void end_renaming(struct renamedata *rd)

{

	unlock_rename(rd->old_parent, rd->new_parent);

fs/overlayfs/dir.c

+29 −14
Original line number Diff line number Diff line
@@ -123,6 +123,7 @@ int ovl_cleanup_and_whiteout(struct ovl_fs *ofs, struct dentry *dir, 
			     struct dentry *dentry)

{

	struct dentry *whiteout;

	struct renamedata rd = {};

	int err;

	int flags = 0;
@@ -134,10 +135,14 @@ int ovl_cleanup_and_whiteout(struct ovl_fs *ofs, struct dentry *dir, 
	if (d_is_dir(dentry))

		flags = RENAME_EXCHANGE;



	err = ovl_lock_rename_workdir(ofs->workdir, whiteout, dir, dentry);

	rd.mnt_idmap = ovl_upper_mnt_idmap(ofs);

	rd.old_parent = ofs->workdir;

	rd.new_parent = dir;

	rd.flags = flags;

	err = start_renaming_two_dentries(&rd, whiteout, dentry);

	if (!err) {

		err = ovl_do_rename(ofs, ofs->workdir, whiteout, dir, dentry, flags);

		unlock_rename(ofs->workdir, dir);

		err = ovl_do_rename_rd(&rd);

		end_renaming(&rd);

	}

	if (err)

		goto kill_whiteout;
@@ -388,6 +393,7 @@ static struct dentry *ovl_clear_empty(struct dentry *dentry, 
	struct ovl_fs *ofs = OVL_FS(dentry->d_sb);

	struct dentry *workdir = ovl_workdir(dentry);

	struct dentry *upperdir = ovl_dentry_upper(dentry->d_parent);

	struct renamedata rd = {};

	struct path upperpath;

	struct dentry *upper;

	struct dentry *opaquedir;
@@ -413,7 +419,11 @@ static struct dentry *ovl_clear_empty(struct dentry *dentry, 
	if (IS_ERR(opaquedir))

		goto out;



	err = ovl_lock_rename_workdir(workdir, opaquedir, upperdir, upper);

	rd.mnt_idmap = ovl_upper_mnt_idmap(ofs);

	rd.old_parent = workdir;

	rd.new_parent = upperdir;

	rd.flags = RENAME_EXCHANGE;

	err = start_renaming_two_dentries(&rd, opaquedir, upper);

	if (err)

		goto out_cleanup_unlocked;
@@ -431,8 +441,8 @@ static struct dentry *ovl_clear_empty(struct dentry *dentry, 
	if (err)

		goto out_cleanup;



	err = ovl_do_rename(ofs, workdir, opaquedir, upperdir, upper, RENAME_EXCHANGE);

	unlock_rename(workdir, upperdir);

	err = ovl_do_rename_rd(&rd);

	end_renaming(&rd);

	if (err)

		goto out_cleanup_unlocked;
@@ -445,7 +455,7 @@ static struct dentry *ovl_clear_empty(struct dentry *dentry, 
	return opaquedir;



out_cleanup:

	unlock_rename(workdir, upperdir);

	end_renaming(&rd);

out_cleanup_unlocked:

	ovl_cleanup(ofs, workdir, opaquedir);

	dput(opaquedir);
@@ -468,6 +478,7 @@ static int ovl_create_over_whiteout(struct dentry *dentry, struct inode *inode, 
	struct ovl_fs *ofs = OVL_FS(dentry->d_sb);

	struct dentry *workdir = ovl_workdir(dentry);

	struct dentry *upperdir = ovl_dentry_upper(dentry->d_parent);

	struct renamedata rd = {};

	struct dentry *upper;

	struct dentry *newdentry;

	int err;
@@ -499,7 +510,11 @@ static int ovl_create_over_whiteout(struct dentry *dentry, struct inode *inode, 
	if (IS_ERR(newdentry))

		goto out_dput;



	err = ovl_lock_rename_workdir(workdir, newdentry, upperdir, upper);

	rd.mnt_idmap = ovl_upper_mnt_idmap(ofs);

	rd.old_parent = workdir;

	rd.new_parent = upperdir;

	rd.flags = 0;

	err = start_renaming_two_dentries(&rd, newdentry, upper);

	if (err)

		goto out_cleanup_unlocked;
@@ -536,16 +551,16 @@ static int ovl_create_over_whiteout(struct dentry *dentry, struct inode *inode, 
		if (err)

			goto out_cleanup;



		err = ovl_do_rename(ofs, workdir, newdentry, upperdir, upper,

				    RENAME_EXCHANGE);

		unlock_rename(workdir, upperdir);

		rd.flags = RENAME_EXCHANGE;

		err = ovl_do_rename_rd(&rd);

		end_renaming(&rd);

		if (err)

			goto out_cleanup_unlocked;



		ovl_cleanup(ofs, workdir, upper);

	} else {

		err = ovl_do_rename(ofs, workdir, newdentry, upperdir, upper, 0);

		unlock_rename(workdir, upperdir);

		err = ovl_do_rename_rd(&rd);

		end_renaming(&rd);

		if (err)

			goto out_cleanup_unlocked;

	}
@@ -565,7 +580,7 @@ static int ovl_create_over_whiteout(struct dentry *dentry, struct inode *inode, 
	return err;



out_cleanup:

	unlock_rename(workdir, upperdir);

	end_renaming(&rd);

out_cleanup_unlocked:

	ovl_cleanup(ofs, workdir, newdentry);

	dput(newdentry);

include/linux/namei.h

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

int start_renaming_two_dentries(struct renamedata *rd,

				struct dentry *old_dentry, struct dentry *new_dentry);

void end_renaming(struct renamedata *rd);



/**

security/selinux/selinuxfs.c

+13 −2
Original line number Diff line number Diff line
@@ -506,6 +506,7 @@ static int sel_make_policy_nodes(struct selinux_fs_info *fsi, 
{

	int ret = 0;

	struct dentry *tmp_parent, *tmp_bool_dir, *tmp_class_dir;

	struct renamedata rd = {};

	unsigned int bool_num = 0;

	char **bool_names = NULL;

	int *bool_values = NULL;
@@ -539,9 +540,14 @@ static int sel_make_policy_nodes(struct selinux_fs_info *fsi, 
	if (ret)

		goto out;



	lock_rename(tmp_parent, fsi->sb->s_root);

	rd.old_parent = tmp_parent;

	rd.new_parent = fsi->sb->s_root;



	/* booleans */

	ret = start_renaming_two_dentries(&rd, tmp_bool_dir, fsi->bool_dir);

	if (ret)

		goto out;



	d_exchange(tmp_bool_dir, fsi->bool_dir);



	swap(fsi->bool_num, bool_num);
@@ -549,12 +555,17 @@ static int sel_make_policy_nodes(struct selinux_fs_info *fsi, 
	swap(fsi->bool_pending_values, bool_values);



	fsi->bool_dir = tmp_bool_dir;

	end_renaming(&rd);



	/* classes */

	ret = start_renaming_two_dentries(&rd, tmp_class_dir, fsi->class_dir);

	if (ret)

		goto out;



	d_exchange(tmp_class_dir, fsi->class_dir);

	fsi->class_dir = tmp_class_dir;



	unlock_rename(tmp_parent, fsi->sb->s_root);

	end_renaming(&rd);



out:

	sel_remove_old_bool_data(bool_num, bool_names, bool_values);