* Re: [PATCH 1/1] NFSD: fix decoding in nfs4_xdr_dec_cb_getattr
2024-12-19 20:12 [PATCH 1/1] NFSD: fix decoding in nfs4_xdr_dec_cb_getattr Olga Kornievskaia
@ 2024-12-19 20:22 ` Jeff Layton
2024-12-19 20:49 ` Benjamin Coddington
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Jeff Layton @ 2024-12-19 20:22 UTC (permalink / raw)
To: Olga Kornievskaia, chuck.lever; +Cc: linux-nfs
On Thu, 2024-12-19 at 15:12 -0500, Olga Kornievskaia wrote:
> If a client were to send an error to a CB_GETATTR call, the code
> erronously continues to try decode past the error code. It ends
> up returning BAD_XDR error to the rpc layer and then in turn
> trigger a WARN_ONCE in nfsd4_cb_done() function.
>
> Fixes: 6487a13b5c6b ("NFSD: add support for CB_GETATTR callback")
> Signed-off-by: Olga Kornievskaia <okorniev@redhat.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 3877b53e429f..f24d8654393d 100644
> --- a/fs/nfsd/nfs4callback.c
> +++ b/fs/nfsd/nfs4callback.c
> @@ -647,7 +647,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 (status)
> + if (status || cb->cb_status)
> return status;
> if (xdr_stream_decode_uint32_array(xdr, bitmap, 3) < 0)
> return -NFSERR_BAD_XDR;
Nice catch!
Reviewed-by: Jeff Layton <jlayton@kernel.org>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH 1/1] NFSD: fix decoding in nfs4_xdr_dec_cb_getattr
2024-12-19 20:12 [PATCH 1/1] NFSD: fix decoding in nfs4_xdr_dec_cb_getattr Olga Kornievskaia
2024-12-19 20:22 ` Jeff Layton
@ 2024-12-19 20:49 ` Benjamin Coddington
2024-12-20 14:32 ` cel
2025-02-10 15:51 ` Jeff Layton
3 siblings, 0 replies; 5+ messages in thread
From: Benjamin Coddington @ 2024-12-19 20:49 UTC (permalink / raw)
To: Olga Kornievskaia; +Cc: chuck.lever, jlayton, linux-nfs
On 19 Dec 2024, at 15:12, Olga Kornievskaia wrote:
> If a client were to send an error to a CB_GETATTR call, the code
> erronously continues to try decode past the error code. It ends
> up returning BAD_XDR error to the rpc layer and then in turn
> trigger a WARN_ONCE in nfsd4_cb_done() function.
>
> Fixes: 6487a13b5c6b ("NFSD: add support for CB_GETATTR callback")
> Signed-off-by: Olga Kornievskaia <okorniev@redhat.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 3877b53e429f..f24d8654393d 100644
> --- a/fs/nfsd/nfs4callback.c
> +++ b/fs/nfsd/nfs4callback.c
> @@ -647,7 +647,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 (status)
> + if (status || cb->cb_status)
> return status;
> if (xdr_stream_decode_uint32_array(xdr, bitmap, 3) < 0)
> return -NFSERR_BAD_XDR;
> --
> 2.43.5
Yep!
Reviewed-by: Benjamin Coddington <bcodding@redhat.com>
Ben
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH 1/1] NFSD: fix decoding in nfs4_xdr_dec_cb_getattr
2024-12-19 20:12 [PATCH 1/1] NFSD: fix decoding in nfs4_xdr_dec_cb_getattr Olga Kornievskaia
2024-12-19 20:22 ` Jeff Layton
2024-12-19 20:49 ` Benjamin Coddington
@ 2024-12-20 14:32 ` cel
2025-02-10 15:51 ` Jeff Layton
3 siblings, 0 replies; 5+ messages in thread
From: cel @ 2024-12-20 14:32 UTC (permalink / raw)
To: jlayton, Olga Kornievskaia; +Cc: Chuck Lever, linux-nfs
From: Chuck Lever <chuck.lever@oracle.com>
On Thu, 19 Dec 2024 15:12:04 -0500, Olga Kornievskaia wrote:
> If a client were to send an error to a CB_GETATTR call, the code
> erronously continues to try decode past the error code. It ends
> up returning BAD_XDR error to the rpc layer and then in turn
> trigger a WARN_ONCE in nfsd4_cb_done() function.
>
>
Applied to nfsd-testing for v6.14, thanks!
[1/1] NFSD: fix decoding in nfs4_xdr_dec_cb_getattr
commit: 06f2bda29525f103e83cbb8a306774d508c7801d
--
Chuck Lever
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH 1/1] NFSD: fix decoding in nfs4_xdr_dec_cb_getattr
2024-12-19 20:12 [PATCH 1/1] NFSD: fix decoding in nfs4_xdr_dec_cb_getattr Olga Kornievskaia
` (2 preceding siblings ...)
2024-12-20 14:32 ` cel
@ 2025-02-10 15:51 ` Jeff Layton
3 siblings, 0 replies; 5+ messages in thread
From: Jeff Layton @ 2025-02-10 15:51 UTC (permalink / raw)
To: Olga Kornievskaia, chuck.lever; +Cc: linux-nfs
On Thu, 2024-12-19 at 15:12 -0500, Olga Kornievskaia wrote:
> If a client were to send an error to a CB_GETATTR call, the code
> erronously continues to try decode past the error code. It ends
> up returning BAD_XDR error to the rpc layer and then in turn
> trigger a WARN_ONCE in nfsd4_cb_done() function.
>
> Fixes: 6487a13b5c6b ("NFSD: add support for CB_GETATTR callback")
> Signed-off-by: Olga Kornievskaia <okorniev@redhat.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 3877b53e429f..f24d8654393d 100644
> --- a/fs/nfsd/nfs4callback.c
> +++ b/fs/nfsd/nfs4callback.c
> @@ -647,7 +647,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 (status)
> + if (status || cb->cb_status)
> return status;
> if (xdr_stream_decode_uint32_array(xdr, bitmap, 3) < 0)
> return -NFSERR_BAD_XDR;
I'm not sure what happened here, but the patch that got committed is
checking cb->cb_seq_status instead of the patch above:
if (unlikely(status || cb->cb_seq_status))
The emailed patch is correct, but the above is bogus (and may be
contributing to the warning reported here:
https://bugzilla.kernel.org/show_bug.cgi?id=219737#c9
--
Jeff Layton <jlayton@kernel.org>
^ permalink raw reply [flat|nested] 5+ messages in thread