2009-06-20 03:13:20

by Labiaga, Ricardo

Subject: [PATCH] nfsd41: Do not renew state on error

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 <[email protected]>
---
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

2009-06-21 14:27:05

by Benny Halevy

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

On Jun. 20, 2009, 6:08 +0300, Ricardo Labiaga <[email protected]> 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 <[email protected]>
> ---
> 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);