2014-06-06 18:56:44

by Jeff Layton

Subject: [PATCH] NFSd: nfs4_preprocess_seqid_op should only set *stpp on success

From: Trond Myklebust <[email protected]>

Signed-off-by: Trond Myklebust <[email protected]>
Signed-off-by: Jeff Layton <[email protected]>
---
fs/nfsd/nfs4state.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index f429883fb4bb..f8c61c2a0a9e 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -3897,6 +3897,7 @@ nfs4_preprocess_seqid_op(struct nfsd4_compound_state *cstate, u32 seqid,
{
__be32 status;
struct nfs4_stid *s;
+ struct nfs4_ol_stateid *stp = NULL;

dprintk("NFSD: %s: seqid=%d stateid = " STATEID_FMT "\n", __func__,
seqid, STATEID_VAL(stateid));
@@ -3906,11 +3907,14 @@ nfs4_preprocess_seqid_op(struct nfsd4_compound_state *cstate, u32 seqid,
cstate->minorversion, nn);
if (status)
return status;
- *stpp = openlockstateid(s);
+ stp = openlockstateid(s);
if (!nfsd4_has_session(cstate))
- cstate->replay_owner = (*stpp)->st_stateowner;
+ cstate->replay_owner = stp->st_stateowner;

- return nfs4_seqid_op_checks(cstate, stateid, seqid, *stpp);
+ status = nfs4_seqid_op_checks(cstate, stateid, seqid, stp);
+ if (!status)
+ *stpp = stp;
+ return status;
}

static __be32 nfs4_preprocess_confirmed_seqid_op(struct nfsd4_compound_state *cstate, u32 seqid,
--
1.9.3

2014-06-07 14:23:29

by Jeff Layton

Subject: Re: [PATCH] NFSd: nfs4_preprocess_seqid_op should only set *stpp on success

On Sat, 7 Jun 2014 07:08:00 -0700
Christoph Hellwig <[email protected]> wrote:

> On Fri, Jun 06, 2014 at 02:56:34PM -0400, Jeff Layton wrote:
> > From: Trond Myklebust <[email protected]>
> >
> > Signed-off-by: Trond Myklebust <[email protected]>
> > Signed-off-by: Jeff Layton <[email protected]>
>
> The new code looks correct, but shouldn't callers already take care
> not to use the returned stateid only if nfs4_preprocess_seqid_op returns
> 0?
>

Yeah, I mentioned that to Bruce on IRC. This is not really a bugfix,
per-se, but more of a defensive coding measure.

--
Jeff Layton <[email protected]>

2014-06-07 14:26:35

by Christoph Hellwig

Subject: Re: [PATCH] NFSd: nfs4_preprocess_seqid_op should only set *stpp on success

On Sat, Jun 07, 2014 at 10:23:26AM -0400, Jeff Layton wrote:
> Yeah, I mentioned that to Bruce on IRC. This is not really a bugfix,
> per-se, but more of a defensive coding measure.

Looks fine to me for that,

Reviewed-by: Christoph Hellwig <[email protected]>

2014-06-07 14:08:01

by Christoph Hellwig

Subject: Re: [PATCH] NFSd: nfs4_preprocess_seqid_op should only set *stpp on success

On Fri, Jun 06, 2014 at 02:56:34PM -0400, Jeff Layton wrote:
> From: Trond Myklebust <[email protected]>
>
> Signed-off-by: Trond Myklebust <[email protected]>
> Signed-off-by: Jeff Layton <[email protected]>

The new code looks correct, but shouldn't callers already take care
not to use the returned stateid only if nfs4_preprocess_seqid_op returns
0?