On Mon, 2025-02-10 at 11:43 -0500, cel@xxxxxxxxxx wrote: > From: Chuck Lever <chuck.lever@xxxxxxxxxx> > > Jeff says: > > Now that I look, 1b3e26a5ccbf is wrong. The patch on the ml was correct, but > the one that got committed is different. It should be: > > status = decode_cb_op_status(xdr, OP_CB_GETATTR, &cb->cb_status); > if (unlikely(status || cb->cb_status)) > > If "status" is non-zero, decoding failed (usu. BADXDR), but we also want to > bail out and not decode the rest of the call if the decoded cb_status is > non-zero. That's not happening here, cb_seq_status has already been checked and > is non-zero, so this ends up trying to decode the rest of the CB_GETATTR reply > when it doesn't exist. > > Reported-by: Jeff Layton: <jlayton@xxxxxxxxxx> > Closes: https://bugzilla.kernel.org/show_bug.cgi?id=219737 > Fixes: 1b3e26a5ccbf ("NFSD: fix decoding in nfs4_xdr_dec_cb_getattr") > Signed-off-by: Chuck Lever <chuck.lever@xxxxxxxxxx> > --- > fs/nfsd/nfs4callback.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c > index cf6d29828f4e..484077200c5d 100644 > --- a/fs/nfsd/nfs4callback.c > +++ b/fs/nfsd/nfs4callback.c > @@ -679,7 +679,7 @@ static int nfs4_xdr_dec_cb_getattr(struct rpc_rqst *rqstp, > return status; > > status = decode_cb_op_status(xdr, OP_CB_GETATTR, &cb->cb_status); > - if (unlikely(status || cb->cb_seq_status)) > + if (unlikely(status || cb->cb_status)) > return status; > if (xdr_stream_decode_uint32_array(xdr, bitmap, 3) < 0) > return -NFSERR_BAD_XDR; Reviewed-by: Jeff Layton <jlayton@xxxxxxxxxx>
