[PATCH] NFSD: Fix CB_GETATTR status fix

[PATCH] NFSD: Fix CB_GETATTR status fix

[cel]

From: Chuck Lever <chuck.lever@oracle.com>

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@kernel.org>
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@oracle.com>
---
 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;
-- 
2.47.0

Re: [PATCH] NFSD: Fix CB_GETATTR status fix

[Jeff Layton]

On Mon, 2025-02-10 at 11:43 -0500, cel@kernel.org wrote:
> From: Chuck Lever <chuck.lever@oracle.com>
> 
> 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@kernel.org>
> 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@oracle.com>
> ---
>  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@kernel.org>