linux-kernel - Re: [PATCH 1/2] NFSv4: Fix the locking in nfs_inode_reclaim

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20100420222140.GN2628@linux.vnet.ibm.com>
Date:	Tue, 20 Apr 2010 15:21:40 -0700
From:	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>
To:	David Howells <dhowells@...hat.com>
Cc:	Trond.Myklebust@...app.com, linux-nfs@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] NFSv4: Fix the locking in
 nfs_inode_reclaim_delegation()

On Tue, Apr 20, 2010 at 11:26:08AM +0100, David Howells wrote:
> From: Trond Myklebust <Trond.Myklebust@...app.com>
> 
> Ensure that we correctly rcu-dereference the delegation itself, and that we
> protect against removal while we're changing the contents.

I queued both of these, thank you Trond and David!

Trond, if you would rather carry these in your tree, please let me know.

							Thanx, Paul

> Signed-off-by: Trond Myklebust <Trond.Myklebust@...app.com>
> Signed-off-by: David Howells <dhowells@...hat.com>
> ---
> 
>  fs/nfs/delegation.c |   42 ++++++++++++++++++++++++++++--------------
>  1 files changed, 28 insertions(+), 14 deletions(-)
> 
> diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c
> index 1567124..8d9ec49 100644
> --- a/fs/nfs/delegation.c
> +++ b/fs/nfs/delegation.c
> @@ -129,21 +129,35 @@ again:
>   */
>  void nfs_inode_reclaim_delegation(struct inode *inode, struct rpc_cred *cred, struct nfs_openres *res)
>  {
> -	struct nfs_delegation *delegation = NFS_I(inode)->delegation;
> -	struct rpc_cred *oldcred;
> +	struct nfs_delegation *delegation;
> +	struct rpc_cred *oldcred = NULL;
> 
> -	if (delegation == NULL)
> -		return;
> -	memcpy(delegation->stateid.data, res->delegation.data,
> -			sizeof(delegation->stateid.data));
> -	delegation->type = res->delegation_type;
> -	delegation->maxsize = res->maxsize;
> -	oldcred = delegation->cred;
> -	delegation->cred = get_rpccred(cred);
> -	clear_bit(NFS_DELEGATION_NEED_RECLAIM, &delegation->flags);
> -	NFS_I(inode)->delegation_state = delegation->type;
> -	smp_wmb();
> -	put_rpccred(oldcred);
> +	rcu_read_lock();
> +	delegation = rcu_dereference(NFS_I(inode)->delegation);
> +	if (delegation != NULL) {
> +		spin_lock(&delegation->lock);
> +		if (delegation->inode != NULL) {
> +			memcpy(delegation->stateid.data, res->delegation.data,
> +			       sizeof(delegation->stateid.data));
> +			delegation->type = res->delegation_type;
> +			delegation->maxsize = res->maxsize;
> +			oldcred = delegation->cred;
> +			delegation->cred = get_rpccred(cred);
> +			clear_bit(NFS_DELEGATION_NEED_RECLAIM,
> +				  &delegation->flags);
> +			NFS_I(inode)->delegation_state = delegation->type;
> +			spin_unlock(&delegation->lock);
> +			put_rpccred(oldcred);
> +			rcu_read_unlock();
> +		} else {
> +			/* We appear to have raced with a delegation return. */
> +			spin_unlock(&delegation->lock);
> +			rcu_read_unlock();
> +			nfs_inode_set_delegation(inode, cred, res);
> +		}
> +	} else {
> +		rcu_read_unlock();
> +	}
>  }
> 
>  static int nfs_do_return_delegation(struct inode *inode, struct nfs_delegation *delegation, int issync)
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/