Re: [PATCH v2] 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 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