Commit 162d0644 authored by Amir Goldstein's avatar Amir Goldstein
Browse files

ovl: reorder ovl_want_write() after ovl_inode_lock() 



Make the locking order of ovl_inode_lock() strictly between the two
vfs stacked layers, i.e.:
- ovl vfs locks: sb_writers, inode_lock, ...
- ovl_inode_lock
- upper vfs locks: sb_writers, inode_lock, ...

To that effect, move ovl_want_write() into the helpers ovl_nlink_start()
and ovl_copy_up_start which currently take the ovl_inode_lock() after
ovl_want_write().

Signed-off-by: default avatarAmir Goldstein <amir73il@gmail.com>
parent d08d3b3c

fs/overlayfs/copy_up.c

+3 −10
Original line number Diff line number Diff line
@@ -1170,17 +1170,10 @@ static bool ovl_open_need_copy_up(struct dentry *dentry, int flags) 


int ovl_maybe_copy_up(struct dentry *dentry, int flags)

{

	int err = 0;



	if (ovl_open_need_copy_up(dentry, flags)) {

		err = ovl_want_write(dentry);

		if (!err) {

			err = ovl_copy_up_flags(dentry, flags);

			ovl_drop_write(dentry);

		}

	}

	if (!ovl_open_need_copy_up(dentry, flags))

		return 0;



	return err;

	return ovl_copy_up_flags(dentry, flags);

}



int ovl_copy_up_with_data(struct dentry *dentry)

fs/overlayfs/dir.c

+26 −34
Original line number Diff line number Diff line
@@ -559,10 +559,6 @@ static int ovl_create_or_link(struct dentry *dentry, struct inode *inode, 
	struct cred *override_cred;

	struct dentry *parent = dentry->d_parent;



	err = ovl_copy_up(parent);

	if (err)

		return err;



	old_cred = ovl_override_creds(dentry->d_sb);



	/*
@@ -626,6 +622,10 @@ static int ovl_create_object(struct dentry *dentry, int mode, dev_t rdev, 
		.link = link,

	};



	err = ovl_copy_up(dentry->d_parent);

	if (err)

		return err;



	err = ovl_want_write(dentry);

	if (err)

		goto out;
@@ -700,28 +700,24 @@ static int ovl_link(struct dentry *old, struct inode *newdir, 
	int err;

	struct inode *inode;



	err = ovl_want_write(old);

	err = ovl_copy_up(old);

	if (err)

		goto out;



	err = ovl_copy_up(old);

	err = ovl_copy_up(new->d_parent);

	if (err)

		goto out_drop_write;

		goto out;



	err = ovl_copy_up(new->d_parent);

	err = ovl_nlink_start(old);

	if (err)

		goto out_drop_write;

		goto out;



	if (ovl_is_metacopy_dentry(old)) {

		err = ovl_set_link_redirect(old);

		if (err)

			goto out_drop_write;

			goto out_nlink_end;

	}



	err = ovl_nlink_start(old);

	if (err)

		goto out_drop_write;



	inode = d_inode(old);

	ihold(inode);
@@ -731,9 +727,8 @@ static int ovl_link(struct dentry *old, struct inode *newdir, 
	if (err)

		iput(inode);



out_nlink_end:

	ovl_nlink_end(old);

out_drop_write:

	ovl_drop_write(old);

out:

	return err;

}
@@ -891,17 +886,13 @@ static int ovl_do_remove(struct dentry *dentry, bool is_dir) 
			goto out;

	}



	err = ovl_want_write(dentry);

	if (err)

		goto out;



	err = ovl_copy_up(dentry->d_parent);

	if (err)

		goto out_drop_write;

		goto out;



	err = ovl_nlink_start(dentry);

	if (err)

		goto out_drop_write;

		goto out;



	old_cred = ovl_override_creds(dentry->d_sb);

	if (!lower_positive)
@@ -926,8 +917,6 @@ static int ovl_do_remove(struct dentry *dentry, bool is_dir) 
	if (ovl_dentry_upper(dentry))

		ovl_copyattr(d_inode(dentry));



out_drop_write:

	ovl_drop_write(dentry);

out:

	ovl_cache_free(&list);

	return err;
@@ -1131,29 +1120,32 @@ static int ovl_rename(struct mnt_idmap *idmap, struct inode *olddir, 
		}

	}



	err = ovl_want_write(old);

	if (err)

		goto out;



	err = ovl_copy_up(old);

	if (err)

		goto out_drop_write;

		goto out;



	err = ovl_copy_up(new->d_parent);

	if (err)

		goto out_drop_write;

		goto out;

	if (!overwrite) {

		err = ovl_copy_up(new);

		if (err)

			goto out_drop_write;

			goto out;

	} else if (d_inode(new)) {

		err = ovl_nlink_start(new);

		if (err)

			goto out_drop_write;

			goto out;



		update_nlink = true;

	}



	if (!update_nlink) {

		/* ovl_nlink_start() took ovl_want_write() */

		err = ovl_want_write(old);

		if (err)

			goto out;

	}



	old_cred = ovl_override_creds(old->d_sb);



	if (!list_empty(&list)) {
@@ -1286,7 +1278,7 @@ static int ovl_rename(struct mnt_idmap *idmap, struct inode *olddir, 
	revert_creds(old_cred);

	if (update_nlink)

		ovl_nlink_end(new);

out_drop_write:

	else

		ovl_drop_write(old);

out:

	dput(opaquedir);

fs/overlayfs/export.c

+1 −6
Original line number Diff line number Diff line
@@ -23,12 +23,7 @@ static int ovl_encode_maybe_copy_up(struct dentry *dentry) 
	if (ovl_dentry_upper(dentry))

		return 0;



	err = ovl_want_write(dentry);

	if (!err) {

	err = ovl_copy_up(dentry);

		ovl_drop_write(dentry);

	}



	if (err) {

		pr_warn_ratelimited("failed to copy up on encode (%pd2, err=%i)\n",

				    dentry, err);

fs/overlayfs/inode.c

+27 −30
Original line number Diff line number Diff line
@@ -32,10 +32,6 @@ int ovl_setattr(struct mnt_idmap *idmap, struct dentry *dentry, 
	if (err)

		return err;



	err = ovl_want_write(dentry);

	if (err)

		goto out;



	if (attr->ia_valid & ATTR_SIZE) {

		/* Truncate should trigger data copy up as well */

		full_copy_up = true;
@@ -54,7 +50,7 @@ int ovl_setattr(struct mnt_idmap *idmap, struct dentry *dentry, 
			winode = d_inode(upperdentry);

			err = get_write_access(winode);

			if (err)

				goto out_drop_write;

				goto out;

		}



		if (attr->ia_valid & (ATTR_KILL_SUID|ATTR_KILL_SGID))
@@ -78,6 +74,10 @@ int ovl_setattr(struct mnt_idmap *idmap, struct dentry *dentry, 
		 */

		attr->ia_valid &= ~ATTR_OPEN;



		err = ovl_want_write(dentry);

		if (err)

			goto out_put_write;



		inode_lock(upperdentry->d_inode);

		old_cred = ovl_override_creds(dentry->d_sb);

		err = ovl_do_notify_change(ofs, upperdentry, attr);
@@ -85,12 +85,12 @@ int ovl_setattr(struct mnt_idmap *idmap, struct dentry *dentry, 
		if (!err)

			ovl_copyattr(dentry->d_inode);

		inode_unlock(upperdentry->d_inode);

		ovl_drop_write(dentry);



out_put_write:

		if (winode)

			put_write_access(winode);

	}

out_drop_write:

	ovl_drop_write(dentry);

out:

	return err;

}
@@ -361,27 +361,27 @@ int ovl_xattr_set(struct dentry *dentry, struct inode *inode, const char *name, 
	struct path realpath;

	const struct cred *old_cred;



	err = ovl_want_write(dentry);

	if (err)

		goto out;



	if (!value && !upperdentry) {

		ovl_path_lower(dentry, &realpath);

		old_cred = ovl_override_creds(dentry->d_sb);

		err = vfs_getxattr(mnt_idmap(realpath.mnt), realdentry, name, NULL, 0);

		revert_creds(old_cred);

		if (err < 0)

			goto out_drop_write;

			goto out;

	}



	if (!upperdentry) {

		err = ovl_copy_up(dentry);

		if (err)

			goto out_drop_write;

			goto out;



		realdentry = ovl_dentry_upper(dentry);

	}



	err = ovl_want_write(dentry);

	if (err)

		goto out;



	old_cred = ovl_override_creds(dentry->d_sb);

	if (value) {

		err = ovl_do_setxattr(ofs, realdentry, name, value, size,
@@ -391,12 +391,10 @@ int ovl_xattr_set(struct dentry *dentry, struct inode *inode, const char *name, 
		err = ovl_do_removexattr(ofs, realdentry, name);

	}

	revert_creds(old_cred);

	ovl_drop_write(dentry);



	/* copy c/mtime */

	ovl_copyattr(inode);



out_drop_write:

	ovl_drop_write(dentry);

out:

	return err;

}
@@ -611,10 +609,6 @@ static int ovl_set_or_remove_acl(struct dentry *dentry, struct inode *inode, 
	struct dentry *upperdentry = ovl_dentry_upper(dentry);

	struct dentry *realdentry = upperdentry ?: ovl_dentry_lower(dentry);



	err = ovl_want_write(dentry);

	if (err)

		return err;



	/*

	 * If ACL is to be removed from a lower file, check if it exists in

	 * the first place before copying it up.
@@ -630,7 +624,7 @@ static int ovl_set_or_remove_acl(struct dentry *dentry, struct inode *inode, 
		revert_creds(old_cred);

		if (IS_ERR(real_acl)) {

			err = PTR_ERR(real_acl);

			goto out_drop_write;

			goto out;

		}

		posix_acl_release(real_acl);

	}
@@ -638,23 +632,26 @@ static int ovl_set_or_remove_acl(struct dentry *dentry, struct inode *inode, 
	if (!upperdentry) {

		err = ovl_copy_up(dentry);

		if (err)

			goto out_drop_write;

			goto out;



		realdentry = ovl_dentry_upper(dentry);

	}



	err = ovl_want_write(dentry);

	if (err)

		goto out;



	old_cred = ovl_override_creds(dentry->d_sb);

	if (acl)

		err = ovl_do_set_acl(ofs, realdentry, acl_name, acl);

	else

		err = ovl_do_remove_acl(ofs, realdentry, acl_name);

	revert_creds(old_cred);

	ovl_drop_write(dentry);



	/* copy c/mtime */

	ovl_copyattr(inode);



out_drop_write:

	ovl_drop_write(dentry);

out:

	return err;

}
@@ -778,14 +775,14 @@ int ovl_fileattr_set(struct mnt_idmap *idmap, 
	unsigned int flags;

	int err;



	err = ovl_want_write(dentry);

	if (err)

		goto out;



	err = ovl_copy_up(dentry);

	if (!err) {

		ovl_path_real(dentry, &upperpath);



		err = ovl_want_write(dentry);

		if (err)

			goto out;



		old_cred = ovl_override_creds(inode->i_sb);

		/*

		 * Store immutable/append-only flags in xattr and clear them
@@ -798,6 +795,7 @@ int ovl_fileattr_set(struct mnt_idmap *idmap, 
		if (!err)

			err = ovl_real_fileattr_set(&upperpath, fa);

		revert_creds(old_cred);

		ovl_drop_write(dentry);



		/*

		 * Merge real inode flags with inode flags read from
@@ -812,7 +810,6 @@ int ovl_fileattr_set(struct mnt_idmap *idmap, 
		/* Update ctime */

		ovl_copyattr(inode);

	}

	ovl_drop_write(dentry);

out:

	return err;

}

fs/overlayfs/util.c

+27 −7
Original line number Diff line number Diff line
@@ -676,16 +676,26 @@ int ovl_copy_up_start(struct dentry *dentry, int flags) 
	int err;



	err = ovl_inode_lock_interruptible(inode);

	if (!err && ovl_already_copied_up_locked(dentry, flags)) {

	if (err)

		return err;



	if (ovl_already_copied_up_locked(dentry, flags))

		err = 1; /* Already copied up */

		ovl_inode_unlock(inode);

	}

	else

		err = ovl_want_write(dentry);

	if (err)

		goto out_unlock;



	return 0;



out_unlock:

	ovl_inode_unlock(inode);

	return err;

}



void ovl_copy_up_end(struct dentry *dentry)

{

	ovl_drop_write(dentry);

	ovl_inode_unlock(d_inode(dentry));

}
@@ -1088,8 +1098,12 @@ int ovl_nlink_start(struct dentry *dentry) 
	if (err)

		return err;



	err = ovl_want_write(dentry);

	if (err)

		goto out_unlock;



	if (d_is_dir(dentry) || !ovl_test_flag(OVL_INDEX, inode))

		goto out;

		return 0;



	old_cred = ovl_override_creds(dentry->d_sb);

	/*
@@ -1100,9 +1114,14 @@ int ovl_nlink_start(struct dentry *dentry) 
	 */

	err = ovl_set_nlink_upper(dentry);

	revert_creds(old_cred);



out:

	if (err)

		goto out_drop_write;



	return 0;



out_drop_write:

	ovl_drop_write(dentry);

out_unlock:

	ovl_inode_unlock(inode);



	return err;
@@ -1120,6 +1139,7 @@ void ovl_nlink_end(struct dentry *dentry) 
		revert_creds(old_cred);

	}



	ovl_drop_write(dentry);

	ovl_inode_unlock(inode);

}