BPF List
 help / color / mirror / Atom feed
* [PATCH v2] libbpf: Limit bitfield dump reads to checked bytes
@ 2026-07-06  2:36 Guangshuo Li
  2026-07-06  2:46 ` sashiko-bot
  2026-07-06 12:31 ` Varun R Mallya
  0 siblings, 2 replies; 3+ messages in thread
From: Guangshuo Li @ 2026-07-06  2:36 UTC (permalink / raw)
  To: Andrii Nakryiko, Eduard Zingerman, Alexei Starovoitov,
	Daniel Borkmann, Kumar Kartikeya Dwivedi, Martin KaFai Lau,
	Song Liu, Yonghong Song, Jiri Olsa, Emil Tsalapatis,
	Varun R Mallya, bpf, linux-kernel
  Cc: Guangshuo Li

btf_dump_get_bitfield_value() bounds-checks the number of bytes covered
by the bitfield, but the value-building loops read the full backing type
size. For narrow bitfields this can read past the checked range.

Checking the full backing type size is too strict for packed bitfields.
For example, a packed structure can contain an 8-bit field backed by an
int type whose BTF size is 4, while the actual object bytes covering the
field are only 1 byte.

Keep the bounds check based on the bytes covered by the field, and make
the value-building loops consume the same checked byte range. Preserve
the full-size read for the non-bitfield path used by unaligned enum
values.

Fixes: 5714ca8cba5e ("libbpf: Fix OOB read in btf_dump_get_bitfield_value")
Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
---
v2:
  - Do not check the full backing type size, as it rejects valid packed
    bitfields.
  - Limit the value-building loops to the checked byte range instead.
  - Preserve the non-bitfield path used for unaligned enum values.

 tools/lib/bpf/btf_dump.c | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/tools/lib/bpf/btf_dump.c b/tools/lib/bpf/btf_dump.c
index cc1ba65bb6c5..5d185f7961ff 100644
--- a/tools/lib/bpf/btf_dump.c
+++ b/tools/lib/bpf/btf_dump.c
@@ -1766,17 +1766,21 @@ static int btf_dump_get_bitfield_value(struct btf_dump *d,
 	__u64 num = 0;
 	int i;
 
-	/* Calculate how many bytes cover the bitfield */
-	start_bit = bits_offset % 8;
-	nr_bytes = (start_bit + bit_sz + 7) / 8;
+	if (bit_sz) {
+		/* Calculate how many bytes cover the bitfield */
+		start_bit = bits_offset % 8;
+		nr_bytes = (start_bit + bit_sz + 7) / 8;
+	} else {
+		nr_bytes = t->size;
+	}
 
 	/* Bound check */
 	if (data + nr_bytes > d->typed_dump->data_end)
 		return -E2BIG;
 
 	/* Maximum supported bitfield size is 64 bits */
-	if (t->size > 8) {
-		pr_warn("unexpected bitfield size %d\n", t->size);
+	if (nr_bytes < 1 || nr_bytes > 8) {
+		pr_warn("unexpected bitfield size %d\n", nr_bytes);
 		return -EINVAL;
 	}
 
@@ -1784,13 +1788,13 @@ static int btf_dump_get_bitfield_value(struct btf_dump *d,
 	 * stored in num, then we left/right shift num to eliminate irrelevant bits.
 	 */
 #if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
-	for (i = t->size - 1; i >= 0; i--)
+	for (i = nr_bytes - 1; i >= 0; i--)
 		num = num * 256 + bytes[i];
-	nr_copy_bits = bit_sz + bits_offset;
+	nr_copy_bits = bit_sz ? bit_sz + bits_offset : nr_bytes * 8;
 #elif __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
-	for (i = 0; i < t->size; i++)
+	for (i = 0; i < nr_bytes; i++)
 		num = num * 256 + bytes[i];
-	nr_copy_bits = t->size * 8 - bits_offset;
+	nr_copy_bits = nr_bytes * 8 - bits_offset;
 #else
 # error "Unrecognized __BYTE_ORDER__"
 #endif
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] libbpf: Limit bitfield dump reads to checked bytes
  2026-07-06  2:36 [PATCH v2] libbpf: Limit bitfield dump reads to checked bytes Guangshuo Li
@ 2026-07-06  2:46 ` sashiko-bot
  2026-07-06 12:31 ` Varun R Mallya
  1 sibling, 0 replies; 3+ messages in thread
From: sashiko-bot @ 2026-07-06  2:46 UTC (permalink / raw)
  To: Guangshuo Li; +Cc: bpf

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The patch introduces a logic error and Undefined Behavior when extracting unaligned enum values on little-endian architectures. When `bit_sz == 0`, `left_shift_bits` is correctly updated to `64 - nr_bytes * 8` (e.g., 32 for a 4-byte enum), but `right_shift_bits` is unconditionally set to `64 - bit_sz = 64`. This results in `(num << 32) >> 64`. Shifting a 64-bit integer by 64 is Undefined Behavior in C. On architectures like x86_64 and ARM64, the shift amount is masked by 63, executing effectively as `>> 0`. Consequently, the extracted enum value remains incorrectly shifted left (e.g., 1 becomes 4294967296), breaking BTF dumping for unaligned enums.
--

commit 0a4db97a6f3bd1db635b97e6e0efd0932b9505b1
Author: Guangshuo Li <lgs201920130244@gmail.com>

libbpf: Limit bitfield dump reads to checked bytes

This patch restricts the number of bytes read during bitfield value
extraction to the actual size of the bitfield, preventing out-of-bounds
reads. It preserves the full-size read for the non-bitfield path.

> diff --git a/tools/lib/bpf/btf_dump.c b/tools/lib/bpf/btf_dump.c
> --- a/tools/lib/bpf/btf_dump.c
> +++ b/tools/lib/bpf/btf_dump.c

[ ... ]

> @@ -1784,13 +1788,13 @@ static int btf_dump_get_bitfield_value(struct btf_dump *d,
>  	 * stored in num, then we left/right shift num to eliminate irrelevant bits.
>  	 */
>  #if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
> -	for (i = t->size - 1; i >= 0; i--)
> +	for (i = nr_bytes - 1; i >= 0; i--)
>  		num = num * 256 + bytes[i];
> -	nr_copy_bits = bit_sz + bits_offset;
> +	nr_copy_bits = bit_sz ? bit_sz + bits_offset : nr_bytes * 8;
>  #elif __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
> -	for (i = 0; i < t->size; i++)
> +	for (i = 0; i < nr_bytes; i++)
>  		num = num * 256 + bytes[i];
> -	nr_copy_bits = t->size * 8 - bits_offset;
> +	nr_copy_bits = nr_bytes * 8 - bits_offset;
>  #else
>  # error "Unrecognized __BYTE_ORDER__"
>  #endif
>  	left_shift_bits = 64 - nr_copy_bits;
>  	right_shift_bits = 64 - bit_sz;
>  
>  	*value = (num << left_shift_bits) >> right_shift_bits;

[Severity: Medium]
Does this introduce undefined behavior when extracting unaligned enum values
where bit_sz is 0?

When bit_sz is 0 in btf_dump_get_bitfield_value(), right_shift_bits is set
to 64. Shifting a 64-bit integer by 64 is undefined behavior in C.

On architectures like x86_64 and arm64, the shift amount is masked by 63,
effectively executing as a shift by 0.

Because left_shift_bits is correctly updated to 64 - nr_bytes * 8
(for example, 32 for a 4-byte enum), the value is shifted left but not
shifted back right.

Can this corrupt the extracted unaligned enum values by leaving them
incorrectly shifted left?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260706023632.625353-1-lgs201920130244@gmail.com?part=1

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] libbpf: Limit bitfield dump reads to checked bytes
  2026-07-06  2:36 [PATCH v2] libbpf: Limit bitfield dump reads to checked bytes Guangshuo Li
  2026-07-06  2:46 ` sashiko-bot
@ 2026-07-06 12:31 ` Varun R Mallya
  1 sibling, 0 replies; 3+ messages in thread
From: Varun R Mallya @ 2026-07-06 12:31 UTC (permalink / raw)
  To: Guangshuo Li
  Cc: Andrii Nakryiko, Eduard Zingerman, Alexei Starovoitov,
	Daniel Borkmann, Kumar Kartikeya Dwivedi, Martin KaFai Lau,
	Song Liu, Yonghong Song, Jiri Olsa, Emil Tsalapatis, bpf,
	linux-kernel

On Mon, Jul 06, 2026 at 10:36:32AM +0800, Guangshuo Li wrote:
> btf_dump_get_bitfield_value() bounds-checks the number of bytes covered
> by the bitfield, but the value-building loops read the full backing type
> size. For narrow bitfields this can read past the checked range.
> 
> Checking the full backing type size is too strict for packed bitfields.
> For example, a packed structure can contain an 8-bit field backed by an
> int type whose BTF size is 4, while the actual object bytes covering the
> field are only 1 byte.
> 
> Keep the bounds check based on the bytes covered by the field, and make
> the value-building loops consume the same checked byte range. Preserve
> the full-size read for the non-bitfield path used by unaligned enum
> values.

This seems like it's heading in the correct direction, but to justify a change
here, could you provide examples that can break the older
implementation, if possible, with an ASAN report ? If it's not really
possible to craft something like that, then this would be an unnecessary
change.

> Fixes: 5714ca8cba5e ("libbpf: Fix OOB read in btf_dump_get_bitfield_value")
> Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
> ---
> v2:
>   - Do not check the full backing type size, as it rejects valid packed
>     bitfields.
>   - Limit the value-building loops to the checked byte range instead.
>   - Preserve the non-bitfield path used for unaligned enum values.
> 
>  tools/lib/bpf/btf_dump.c | 22 +++++++++++++---------
>  1 file changed, 13 insertions(+), 9 deletions(-)
> 
> diff --git a/tools/lib/bpf/btf_dump.c b/tools/lib/bpf/btf_dump.c
> index cc1ba65bb6c5..5d185f7961ff 100644
> --- a/tools/lib/bpf/btf_dump.c
> +++ b/tools/lib/bpf/btf_dump.c
> @@ -1766,17 +1766,21 @@ static int btf_dump_get_bitfield_value(struct btf_dump *d,
>  	__u64 num = 0;
>  	int i;
>  
> -	/* Calculate how many bytes cover the bitfield */
> -	start_bit = bits_offset % 8;
> -	nr_bytes = (start_bit + bit_sz + 7) / 8;
> +	if (bit_sz) {
> +		/* Calculate how many bytes cover the bitfield */
> +		start_bit = bits_offset % 8;
> +		nr_bytes = (start_bit + bit_sz + 7) / 8;
> +	} else {
> +		nr_bytes = t->size;
> +	}
>  
>  	/* Bound check */
>  	if (data + nr_bytes > d->typed_dump->data_end)
>  		return -E2BIG;
>  
>  	/* Maximum supported bitfield size is 64 bits */
> -	if (t->size > 8) {
> -		pr_warn("unexpected bitfield size %d\n", t->size);
> +	if (nr_bytes < 1 || nr_bytes > 8) {
> +		pr_warn("unexpected bitfield size %d\n", nr_bytes);
>  		return -EINVAL;
>  	}
>  
> @@ -1784,13 +1788,13 @@ static int btf_dump_get_bitfield_value(struct btf_dump *d,
>  	 * stored in num, then we left/right shift num to eliminate irrelevant bits.
>  	 */
>  #if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
> -	for (i = t->size - 1; i >= 0; i--)
> +	for (i = nr_bytes - 1; i >= 0; i--)
>  		num = num * 256 + bytes[i];
> -	nr_copy_bits = bit_sz + bits_offset;
> +	nr_copy_bits = bit_sz ? bit_sz + bits_offset : nr_bytes * 8;
>  #elif __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
> -	for (i = 0; i < t->size; i++)
> +	for (i = 0; i < nr_bytes; i++)
>  		num = num * 256 + bytes[i];
> -	nr_copy_bits = t->size * 8 - bits_offset;
> +	nr_copy_bits = nr_bytes * 8 - bits_offset;
>  #else
>  # error "Unrecognized __BYTE_ORDER__"
>  #endif
> -- 
> 2.43.0
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-07-06 12:32 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-06  2:36 [PATCH v2] libbpf: Limit bitfield dump reads to checked bytes Guangshuo Li
2026-07-06  2:46 ` sashiko-bot
2026-07-06 12:31 ` Varun R Mallya

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox