Commit 73ca1001 authored by Jeff Layton's avatar Jeff Layton Committed by Trond Myklebust
Browse files

nfs: don't use d_move in nfs_async_rename_done 



If the task that initiated the sillyrename ends up being killed by a
fatal signal, then it will eventually return back to userspace and end
up releasing the i_mutex. d_move however needs to be done while holding
the i_mutex.

Instead of using d_move here, just unhash the old and new dentries to
prevent them from being found by lookups. With this change though, the
dentries are now incorrect post-rename and do not reflect the actual
name of the file on the server. I'm proceeding under the assumption
that since they are unhashed that this isn't really a problem.

In order for the sillydelete to still work though, the dname must be
copied earlier when setting up the sillydelete info, and the name must
be recopied if the sillydelete info has to be moved to a new dentry.

Reported-by: default avatarAl Viro <viro@ZenIV.linux.org.uk>
Signed-off-by: default avatarJeff Layton <jlayton@redhat.com>
Signed-off-by: default avatarTrond Myklebust <Trond.Myklebust@netapp.com>
parent 2773395b

fs/nfs/unlink.c

+19 −10
Original line number Original line Diff line number Diff line
@@ -147,7 +147,7 @@ static int nfs_do_call_unlink(struct dentry *parent, struct inode *dir, struct n 




	alias = d_lookup(parent, &data->args.name);

	alias = d_lookup(parent, &data->args.name);

	if (alias != NULL) {

	if (alias != NULL) {

		int ret = 0;

		int ret;

		void *devname_garbage = NULL;

		void *devname_garbage = NULL;





		/*

		/*
@@ -155,14 +155,16 @@ static int nfs_do_call_unlink(struct dentry *parent, struct inode *dir, struct n 
		 * the sillyrename information to the aliased dentry.

		 * the sillyrename information to the aliased dentry.

		 */

		 */

		nfs_free_dname(data);

		nfs_free_dname(data);

		ret = nfs_copy_dname(alias, data);

		spin_lock(&alias->d_lock);

		spin_lock(&alias->d_lock);

		if (alias->d_inode != NULL &&

		if (ret == 0 && alias->d_inode != NULL &&

		    !(alias->d_flags & DCACHE_NFSFS_RENAMED)) {

		    !(alias->d_flags & DCACHE_NFSFS_RENAMED)) {

			devname_garbage = alias->d_fsdata;

			devname_garbage = alias->d_fsdata;

			alias->d_fsdata = data;

			alias->d_fsdata = data;

			alias->d_flags |= DCACHE_NFSFS_RENAMED;

			alias->d_flags |= DCACHE_NFSFS_RENAMED;

			ret = 1;

			ret = 1;

		}

		} else

			ret = 0;

		spin_unlock(&alias->d_lock);

		spin_unlock(&alias->d_lock);

		nfs_dec_sillycount(dir);

		nfs_dec_sillycount(dir);

		dput(alias);

		dput(alias);
@@ -171,7 +173,6 @@ static int nfs_do_call_unlink(struct dentry *parent, struct inode *dir, struct n 
		 * point dentry is definitely not a root, so we won't need

		 * point dentry is definitely not a root, so we won't need

		 * that anymore.

		 * that anymore.

		 */

		 */

		if (devname_garbage)

		kfree(devname_garbage);

		kfree(devname_garbage);

		return ret;

		return ret;

	}

	}
@@ -204,8 +205,6 @@ static int nfs_call_unlink(struct dentry *dentry, struct nfs_unlinkdata *data) 
	if (parent == NULL)

	if (parent == NULL)

		goto out_free;

		goto out_free;

	dir = parent->d_inode;

	dir = parent->d_inode;

	if (nfs_copy_dname(dentry, data) != 0)

		goto out_dput;

	/* Non-exclusive lock protects against concurrent lookup() calls */

	/* Non-exclusive lock protects against concurrent lookup() calls */

	spin_lock(&dir->i_lock);

	spin_lock(&dir->i_lock);

	if (atomic_inc_not_zero(&NFS_I(dir)->silly_count) == 0) {

	if (atomic_inc_not_zero(&NFS_I(dir)->silly_count) == 0) {
@@ -366,6 +365,8 @@ static void nfs_async_rename_done(struct rpc_task *task, void *calldata) 
	struct nfs_renamedata *data = calldata;

	struct nfs_renamedata *data = calldata;

	struct inode *old_dir = data->old_dir;

	struct inode *old_dir = data->old_dir;

	struct inode *new_dir = data->new_dir;

	struct inode *new_dir = data->new_dir;

	struct dentry *old_dentry = data->old_dentry;

	struct dentry *new_dentry = data->new_dentry;





	if (!NFS_PROTO(old_dir)->rename_done(task, old_dir, new_dir)) {

	if (!NFS_PROTO(old_dir)->rename_done(task, old_dir, new_dir)) {

		nfs_restart_rpc(task, NFS_SERVER(old_dir)->nfs_client);

		nfs_restart_rpc(task, NFS_SERVER(old_dir)->nfs_client);
@@ -373,12 +374,12 @@ static void nfs_async_rename_done(struct rpc_task *task, void *calldata) 
	}

	}





	if (task->tk_status != 0) {

	if (task->tk_status != 0) {

		nfs_cancel_async_unlink(data->old_dentry);

		nfs_cancel_async_unlink(old_dentry);

		return;

		return;

	}

	}





	nfs_set_verifier(data->old_dentry, nfs_save_change_attribute(old_dir));

	d_drop(old_dentry);

	d_move(data->old_dentry, data->new_dentry);

	d_drop(new_dentry);

}

}





/**

/**
@@ -568,6 +569,14 @@ nfs_sillyrename(struct inode *dir, struct dentry *dentry) 
	if (error)

	if (error)

		goto out_dput;

		goto out_dput;





	/* populate unlinkdata with the right dname */

	error = nfs_copy_dname(sdentry,

				(struct nfs_unlinkdata *)dentry->d_fsdata);

	if (error) {

		nfs_cancel_async_unlink(dentry);

		goto out_dput;

	}



	/* run the rename task, undo unlink if it fails */

	/* run the rename task, undo unlink if it fails */

	task = nfs_async_rename(dir, dir, dentry, sdentry);

	task = nfs_async_rename(dir, dir, dentry, sdentry);

	if (IS_ERR(task)) {

	if (IS_ERR(task)) {

CRA Git | Maintained and supported by SUSTech CRA and CCSE