Re: [PATCH] libbpf: Limit bitfield dump reads to checked bytes

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The patch introduces data corruption and undefined behavior when reading unaligned enums on Little Endian architectures by shifting a 64-bit integer by 64 bits.
--

--- Patch [1]: [PATCH] libbpf: Limit bitfield dump reads to checked bytes ---
commit 128a41a5de9f0bf022f4b652bb58245609c3162a
Author: Guangshuo Li <lgs201920130244@gmail.com>
libbpf: Limit bitfield dump reads to checked bytes

This commit updates btf_dump_get_bitfield_value() to limit the bitfield
value-building loops to the same checked byte range instead of the full
backing type size. It aims to prevent reading past the checked range for
narrow bitfields.

> 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

[Severity: Medium]
Does this change introduce data corruption for unaligned enums on Little
Endian architectures?

When bit_sz is 0 (for unaligned enums), nr_copy_bits now becomes
nr_bytes * 8 (e.g., 32 for a 4-byte enum).

The code immediately following this block in btf_dump_get_bitfield_value()
calculates:

	left_shift_bits = 64 - nr_copy_bits;
	right_shift_bits = 64 - bit_sz;

This results in left_shift_bits = 32, and right_shift_bits = 64.
The final shift evaluates to:

	*value = (num << 32) >> 64;

Shifting a 64-bit integer by 64 bits is undefined behavior. On architectures
like x86 and ARM64, the shift amount is masked (modulo 64), meaning >> 64
executes as >> 0.

This results in (num << 32) >> 0, returning a corrupted value instead
of the expected unaligned enum value.

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