* [PATCH] SUNRPC: xdrgen: Initialize data pointer for zero-length items
@ 2025-12-20 15:41 Chuck Lever
2025-12-22 12:47 ` Jeff Layton
2025-12-27 4:49 ` NeilBrown
0 siblings, 2 replies; 3+ messages in thread
From: Chuck Lever @ 2025-12-20 15:41 UTC (permalink / raw)
To: NeilBrown, Jeff Layton, Olga Kornievskaia, Dai Ngo, Tom Talpey
Cc: linux-nfs, Chuck Lever
From: Chuck Lever <chuck.lever@oracle.com>
The xdrgen decoders for strings and opaque data had an
optimization that skipped calling xdr_inline_decode() when the
item length was zero. This left the data pointer uninitialized,
which could lead to unpredictable behavior when callers access
it.
Remove the zero-length check and always call xdr_inline_decode().
When passed a length of zero, xdr_inline_decode() returns the
current buffer position, which is valid and matches the behavior
of hand-coded XDR decoders throughout the kernel.
Fixes: 4b132aacb076 ("tools: Add xdrgen")
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
include/linux/sunrpc/xdrgen/_builtins.h | 20 ++++++++------------
1 file changed, 8 insertions(+), 12 deletions(-)
diff --git a/include/linux/sunrpc/xdrgen/_builtins.h b/include/linux/sunrpc/xdrgen/_builtins.h
index 52ed9a9151c4..a723fb1da9c8 100644
--- a/include/linux/sunrpc/xdrgen/_builtins.h
+++ b/include/linux/sunrpc/xdrgen/_builtins.h
@@ -248,12 +248,10 @@ xdrgen_decode_string(struct xdr_stream *xdr, string *ptr, u32 maxlen)
return false;
if (unlikely(maxlen && len > maxlen))
return false;
- if (len != 0) {
- p = xdr_inline_decode(xdr, len);
- if (unlikely(!p))
- return false;
- ptr->data = (unsigned char *)p;
- }
+ p = xdr_inline_decode(xdr, len);
+ if (unlikely(!p))
+ return false;
+ ptr->data = (unsigned char *)p;
ptr->len = len;
return true;
}
@@ -279,12 +277,10 @@ xdrgen_decode_opaque(struct xdr_stream *xdr, opaque *ptr, u32 maxlen)
return false;
if (unlikely(maxlen && len > maxlen))
return false;
- if (len != 0) {
- p = xdr_inline_decode(xdr, len);
- if (unlikely(!p))
- return false;
- ptr->data = (u8 *)p;
- }
+ p = xdr_inline_decode(xdr, len);
+ if (unlikely(!p))
+ return false;
+ ptr->data = (u8 *)p;
ptr->len = len;
return true;
}
--
2.52.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] SUNRPC: xdrgen: Initialize data pointer for zero-length items
2025-12-20 15:41 [PATCH] SUNRPC: xdrgen: Initialize data pointer for zero-length items Chuck Lever
@ 2025-12-22 12:47 ` Jeff Layton
2025-12-27 4:49 ` NeilBrown
1 sibling, 0 replies; 3+ messages in thread
From: Jeff Layton @ 2025-12-22 12:47 UTC (permalink / raw)
To: Chuck Lever, NeilBrown, Olga Kornievskaia, Dai Ngo, Tom Talpey
Cc: linux-nfs, Chuck Lever
On Sat, 2025-12-20 at 10:41 -0500, Chuck Lever wrote:
> From: Chuck Lever <chuck.lever@oracle.com>
>
> The xdrgen decoders for strings and opaque data had an
> optimization that skipped calling xdr_inline_decode() when the
> item length was zero. This left the data pointer uninitialized,
> which could lead to unpredictable behavior when callers access
> it.
>
> Remove the zero-length check and always call xdr_inline_decode().
> When passed a length of zero, xdr_inline_decode() returns the
> current buffer position, which is valid and matches the behavior
> of hand-coded XDR decoders throughout the kernel.
>
> Fixes: 4b132aacb076 ("tools: Add xdrgen")
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
> include/linux/sunrpc/xdrgen/_builtins.h | 20 ++++++++------------
> 1 file changed, 8 insertions(+), 12 deletions(-)
>
> diff --git a/include/linux/sunrpc/xdrgen/_builtins.h b/include/linux/sunrpc/xdrgen/_builtins.h
> index 52ed9a9151c4..a723fb1da9c8 100644
> --- a/include/linux/sunrpc/xdrgen/_builtins.h
> +++ b/include/linux/sunrpc/xdrgen/_builtins.h
> @@ -248,12 +248,10 @@ xdrgen_decode_string(struct xdr_stream *xdr, string *ptr, u32 maxlen)
> return false;
> if (unlikely(maxlen && len > maxlen))
> return false;
> - if (len != 0) {
> - p = xdr_inline_decode(xdr, len);
> - if (unlikely(!p))
> - return false;
> - ptr->data = (unsigned char *)p;
> - }
> + p = xdr_inline_decode(xdr, len);
> + if (unlikely(!p))
> + return false;
> + ptr->data = (unsigned char *)p;
> ptr->len = len;
> return true;
> }
> @@ -279,12 +277,10 @@ xdrgen_decode_opaque(struct xdr_stream *xdr, opaque *ptr, u32 maxlen)
> return false;
> if (unlikely(maxlen && len > maxlen))
> return false;
> - if (len != 0) {
> - p = xdr_inline_decode(xdr, len);
> - if (unlikely(!p))
> - return false;
> - ptr->data = (u8 *)p;
> - }
> + p = xdr_inline_decode(xdr, len);
> + if (unlikely(!p))
> + return false;
> + ptr->data = (u8 *)p;
> ptr->len = len;
> return true;
> }
Reviewed-by: Jeff Layton <jlayton@kernel.org>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] SUNRPC: xdrgen: Initialize data pointer for zero-length items
2025-12-20 15:41 [PATCH] SUNRPC: xdrgen: Initialize data pointer for zero-length items Chuck Lever
2025-12-22 12:47 ` Jeff Layton
@ 2025-12-27 4:49 ` NeilBrown
1 sibling, 0 replies; 3+ messages in thread
From: NeilBrown @ 2025-12-27 4:49 UTC (permalink / raw)
To: Chuck Lever
Cc: Jeff Layton, Olga Kornievskaia, Dai Ngo, Tom Talpey, linux-nfs,
Chuck Lever
On Sun, 21 Dec 2025, Chuck Lever wrote:
> From: Chuck Lever <chuck.lever@oracle.com>
>
> The xdrgen decoders for strings and opaque data had an
> optimization that skipped calling xdr_inline_decode() when the
> item length was zero. This left the data pointer uninitialized,
> which could lead to unpredictable behavior when callers access
> it.
>
> Remove the zero-length check and always call xdr_inline_decode().
> When passed a length of zero, xdr_inline_decode() returns the
> current buffer position, which is valid and matches the behavior
> of hand-coded XDR decoders throughout the kernel.
>
> Fixes: 4b132aacb076 ("tools: Add xdrgen")
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
> include/linux/sunrpc/xdrgen/_builtins.h | 20 ++++++++------------
> 1 file changed, 8 insertions(+), 12 deletions(-)
>
> diff --git a/include/linux/sunrpc/xdrgen/_builtins.h b/include/linux/sunrpc/xdrgen/_builtins.h
> index 52ed9a9151c4..a723fb1da9c8 100644
> --- a/include/linux/sunrpc/xdrgen/_builtins.h
> +++ b/include/linux/sunrpc/xdrgen/_builtins.h
> @@ -248,12 +248,10 @@ xdrgen_decode_string(struct xdr_stream *xdr, string *ptr, u32 maxlen)
> return false;
> if (unlikely(maxlen && len > maxlen))
> return false;
> - if (len != 0) {
> - p = xdr_inline_decode(xdr, len);
> - if (unlikely(!p))
> - return false;
> - ptr->data = (unsigned char *)p;
> - }
> + p = xdr_inline_decode(xdr, len);
> + if (unlikely(!p))
> + return false;
> + ptr->data = (unsigned char *)p;
> ptr->len = len;
> return true;
> }
> @@ -279,12 +277,10 @@ xdrgen_decode_opaque(struct xdr_stream *xdr, opaque *ptr, u32 maxlen)
> return false;
> if (unlikely(maxlen && len > maxlen))
> return false;
> - if (len != 0) {
> - p = xdr_inline_decode(xdr, len);
> - if (unlikely(!p))
> - return false;
> - ptr->data = (u8 *)p;
> - }
> + p = xdr_inline_decode(xdr, len);
> + if (unlikely(!p))
> + return false;
> + ptr->data = (u8 *)p;
> ptr->len = len;
> return true;
> }
Much nicer. Some "optimisations" really aren't!
Reviewed-by: NeilBrown <neil@brown.name>
Thanks,
NeilBrown
> --
> 2.52.0
>
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-12-27 4:49 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-20 15:41 [PATCH] SUNRPC: xdrgen: Initialize data pointer for zero-length items Chuck Lever
2025-12-22 12:47 ` Jeff Layton
2025-12-27 4:49 ` NeilBrown
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox