[PATCH] NFSD: Fix callback decoder status codes

[PATCH] NFSD: Fix callback decoder status codes

[cel]

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

fs/nfsd/nfs4callback.c implements a callback client. Thus its XDR
decoders are decoding replies, not calls.

NFS4ERR_BAD_XDR is an on-the-wire status code that reports that the
client sent a corrupted RPC /call/. It's not used as the internal
error code when a /reply/ can't be decoded, since that kind of
failure is never reported to the sender of that RPC message.

Instead, a reply decoder should return -EIO, as the reply decoders
in the NFS client do.

Fixes: 6487a13b5c6b ("NFSD: add support for CB_GETATTR callback")
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 fs/nfsd/nfs4callback.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

This is only compile-tested, but I'm thinking of throwing this on
the "for v6.15" pile.


diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c
index 961b1defcb1a..ae4b7b6df47f 100644
--- a/fs/nfsd/nfs4callback.c
+++ b/fs/nfsd/nfs4callback.c
@@ -101,15 +101,15 @@ static int decode_cb_fattr4(struct xdr_stream *xdr, uint32_t *bitmap,
 
 	if (bitmap[0] & FATTR4_WORD0_CHANGE)
 		if (xdr_stream_decode_u64(xdr, &fattr->ncf_cb_change) < 0)
-			return -NFSERR_BAD_XDR;
+			return -EIO;
 	if (bitmap[0] & FATTR4_WORD0_SIZE)
 		if (xdr_stream_decode_u64(xdr, &fattr->ncf_cb_fsize) < 0)
-			return -NFSERR_BAD_XDR;
+			return -EIO;
 	if (bitmap[2] & FATTR4_WORD2_TIME_DELEG_ACCESS) {
 		fattr4_time_deleg_access access;
 
 		if (!xdrgen_decode_fattr4_time_deleg_access(xdr, &access))
-			return -NFSERR_BAD_XDR;
+			return -EIO;
 		fattr->ncf_cb_atime.tv_sec = access.seconds;
 		fattr->ncf_cb_atime.tv_nsec = access.nseconds;
 
@@ -118,7 +118,7 @@ static int decode_cb_fattr4(struct xdr_stream *xdr, uint32_t *bitmap,
 		fattr4_time_deleg_modify modify;
 
 		if (!xdrgen_decode_fattr4_time_deleg_modify(xdr, &modify))
-			return -NFSERR_BAD_XDR;
+			return -EIO;
 		fattr->ncf_cb_mtime.tv_sec = modify.seconds;
 		fattr->ncf_cb_mtime.tv_nsec = modify.nseconds;
 
@@ -682,15 +682,15 @@ static int nfs4_xdr_dec_cb_getattr(struct rpc_rqst *rqstp,
 	if (unlikely(status || cb->cb_status))
 		return status;
 	if (xdr_stream_decode_uint32_array(xdr, bitmap, 3) < 0)
-		return -NFSERR_BAD_XDR;
+		return -EIO;
 	if (xdr_stream_decode_u32(xdr, &attrlen) < 0)
-		return -NFSERR_BAD_XDR;
+		return -EIO;
 	maxlen = sizeof(ncf->ncf_cb_change) + sizeof(ncf->ncf_cb_fsize);
 	if (bitmap[2] != 0)
 		maxlen += (sizeof(ncf->ncf_cb_mtime.tv_sec) +
 			   sizeof(ncf->ncf_cb_mtime.tv_nsec)) * 2;
 	if (attrlen > maxlen)
-		return -NFSERR_BAD_XDR;
+		return -EIO;
 	status = decode_cb_fattr4(xdr, bitmap, ncf);
 	return status;
 }
-- 
2.47.0

Re: [PATCH] NFSD: Fix callback decoder status codes

[Jeff Layton]

On Mon, 2025-02-10 at 12:03 -0500, cel@kernel.org wrote:
> From: Chuck Lever <chuck.lever@oracle.com>
> 
> fs/nfsd/nfs4callback.c implements a callback client. Thus its XDR
> decoders are decoding replies, not calls.
> 
> NFS4ERR_BAD_XDR is an on-the-wire status code that reports that the
> client sent a corrupted RPC /call/. It's not used as the internal
> error code when a /reply/ can't be decoded, since that kind of
> failure is never reported to the sender of that RPC message.
> 
> Instead, a reply decoder should return -EIO, as the reply decoders
> in the NFS client do.
> 
> Fixes: 6487a13b5c6b ("NFSD: add support for CB_GETATTR callback")
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
>  fs/nfsd/nfs4callback.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> This is only compile-tested, but I'm thinking of throwing this on
> the "for v6.15" pile.
> 
> 
> diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c
> index 961b1defcb1a..ae4b7b6df47f 100644
> --- a/fs/nfsd/nfs4callback.c
> +++ b/fs/nfsd/nfs4callback.c
> @@ -101,15 +101,15 @@ static int decode_cb_fattr4(struct xdr_stream *xdr, uint32_t *bitmap,
>  
>  	if (bitmap[0] & FATTR4_WORD0_CHANGE)
>  		if (xdr_stream_decode_u64(xdr, &fattr->ncf_cb_change) < 0)
> -			return -NFSERR_BAD_XDR;
> +			return -EIO;
>  	if (bitmap[0] & FATTR4_WORD0_SIZE)
>  		if (xdr_stream_decode_u64(xdr, &fattr->ncf_cb_fsize) < 0)
> -			return -NFSERR_BAD_XDR;
> +			return -EIO;
>  	if (bitmap[2] & FATTR4_WORD2_TIME_DELEG_ACCESS) {
>  		fattr4_time_deleg_access access;
>  
>  		if (!xdrgen_decode_fattr4_time_deleg_access(xdr, &access))
> -			return -NFSERR_BAD_XDR;
> +			return -EIO;
>  		fattr->ncf_cb_atime.tv_sec = access.seconds;
>  		fattr->ncf_cb_atime.tv_nsec = access.nseconds;
>  
> @@ -118,7 +118,7 @@ static int decode_cb_fattr4(struct xdr_stream *xdr, uint32_t *bitmap,
>  		fattr4_time_deleg_modify modify;
>  
>  		if (!xdrgen_decode_fattr4_time_deleg_modify(xdr, &modify))
> -			return -NFSERR_BAD_XDR;
> +			return -EIO;
>  		fattr->ncf_cb_mtime.tv_sec = modify.seconds;
>  		fattr->ncf_cb_mtime.tv_nsec = modify.nseconds;
>  
> @@ -682,15 +682,15 @@ static int nfs4_xdr_dec_cb_getattr(struct rpc_rqst *rqstp,
>  	if (unlikely(status || cb->cb_status))
>  		return status;
>  	if (xdr_stream_decode_uint32_array(xdr, bitmap, 3) < 0)
> -		return -NFSERR_BAD_XDR;
> +		return -EIO;
>  	if (xdr_stream_decode_u32(xdr, &attrlen) < 0)
> -		return -NFSERR_BAD_XDR;
> +		return -EIO;
>  	maxlen = sizeof(ncf->ncf_cb_change) + sizeof(ncf->ncf_cb_fsize);
>  	if (bitmap[2] != 0)
>  		maxlen += (sizeof(ncf->ncf_cb_mtime.tv_sec) +
>  			   sizeof(ncf->ncf_cb_mtime.tv_nsec)) * 2;
>  	if (attrlen > maxlen)
> -		return -NFSERR_BAD_XDR;
> +		return -EIO;
>  	status = decode_cb_fattr4(xdr, bitmap, ncf);
>  	return status;
>  }

Reviewed-by: Jeff Layton <jlayton@kernel.org>