[PATCH] nfsd41: Do not renew state on error

[PATCH] nfsd41: Do not renew state on error

[Ricardo Labiaga]

nfsd4_sequence() should not renew the client state if the session was not
found or if there was a bad slot.  This will also avoid dereferencing a
null session pointer.

Signed-off-by: Ricardo Labiaga <Ricardo.Labiaga@netapp.com>
---
 fs/nfsd/nfs4state.c |   12 ++++++++----
 1 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index c845365..e239c6e 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -1425,12 +1425,16 @@ nfsd4_sequence(struct svc_rqst *rqstp,
 	spin_lock(&sessionid_lock);
 	status = nfserr_badsession;
 	session = find_in_sessionid_hashtbl(&seq->sessionid);
-	if (!session)
-		goto out;
+	if (!session) {
+		spin_unlock(&sessionid_lock);
+		goto err;
+	}
 
 	status = nfserr_badslot;
-	if (seq->slotid >= session->se_fchannel.maxreqs)
-		goto out;
+	if (seq->slotid >= session->se_fchannel.maxreqs) {
+		spin_unlock(&sessionid_lock);
+		goto err;
+	}
 
 	slot = &session->se_slots[seq->slotid];
 	dprintk("%s: slotid %d\n", __func__, seq->slotid);
-- 
1.5.4.3

Re: [pnfs] [PATCH] nfsd41: Do not renew state on error

[Benny Halevy]

On Jun. 20, 2009, 6:08 +0300, Ricardo Labiaga <Ricardo.Labiaga@netapp.com> wrote:
> nfsd4_sequence() should not renew the client state if the session was not
> found or if there was a bad slot.  This will also avoid dereferencing a
> null session pointer.

Thanks.  I'll merge that onto my nfsd41-for-2.6.31 branch.
I was thinking whether the spin_unlock should be done on
the err: label and separate it from the success path.
I'll look into that along with reworking the state lock
as renewing the client (or actually marking it for renew)
will no longer need the lock.

Benny

> 
> Signed-off-by: Ricardo Labiaga <Ricardo.Labiaga@netapp.com>
> ---
>  fs/nfsd/nfs4state.c |   12 ++++++++----
>  1 files changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index c845365..e239c6e 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -1425,12 +1425,16 @@ nfsd4_sequence(struct svc_rqst *rqstp,
>  	spin_lock(&sessionid_lock);
>  	status = nfserr_badsession;
>  	session = find_in_sessionid_hashtbl(&seq->sessionid);
> -	if (!session)
> -		goto out;
> +	if (!session) {
> +		spin_unlock(&sessionid_lock);
> +		goto err;
> +	}
>  
>  	status = nfserr_badslot;
> -	if (seq->slotid >= session->se_fchannel.maxreqs)
> -		goto out;
> +	if (seq->slotid >= session->se_fchannel.maxreqs) {
> +		spin_unlock(&sessionid_lock);
> +		goto err;
> +	}
>  
>  	slot = &session->se_slots[seq->slotid];
>  	dprintk("%s: slotid %d\n", __func__, seq->slotid);