Re: [PATCH bpf-next] libbpf: Fix sign extension of unaligned enums in btf_dump_get_enum_value()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] libbpf: false -E2BIG on unaligned enum bitfields in
  btf_dump_get_enum_value()
It also noticed 1 related pre-existing issue this patch does not
introduce:
- [Medium] libbpf: out-of-bounds read in btf_dump_type_data_check_zero()
  [1]
--

commit 7eb03a62b6fbbb96cde677bedc0b0fa89d38e668
Author: Hungyu Lin <dennylin0707@gmail.com>

libbpf: Fix sign extension of unaligned enums in btf_dump_get_enum_value()

This commit fixes sign extension for unaligned enum values in
btf_dump_get_enum_value() by copying the unaligned value into a local
aligned buffer before reading it with the same signed loads used for
aligned data.

> diff --git a/tools/lib/bpf/btf_dump.c b/tools/lib/bpf/btf_dump.c
> index bbe86928b3eca..30fe3bb48df6c 100644
> --- a/tools/lib/bpf/btf_dump.c
> +++ b/tools/lib/bpf/btf_dump.c
> @@ -2329,16 +2329,13 @@ static int btf_dump_get_enum_value(struct btf_dump *d,
>  				   __s64 *value)
>  {
>  	bool is_signed = btf_kflag(t);
> +	char buf[8] __attribute__((aligned(8)));
>  
> -	if (!ptr_is_aligned(d->btf, id, data)) {
> -		__u64 val;
> -		int err;
> -
> -		err = btf_dump_get_bitfield_value(d, t, data, 0, 0, &val);
> -		if (err)
> -			return err;
> -		*value = (__s64)val;
> -		return 0;
> +	if (t->size <= sizeof(buf) && !ptr_is_aligned(d->btf, id, data)) {
> +		if (data + t->size > d->typed_dump->data_end)
> +			return -E2BIG;
> +		memcpy(buf, data, t->size);
> +		data = buf;
>  	}

[Severity: Medium]
Can this return a false -E2BIG regression when dumping a struct that has
an unaligned enum bitfield within the last t->size - 1 bytes of the
buffer (for example, enum fs_context_phase phase:8 in struct fs_context)?

When emit_zeroes is false, btf_dump_type_data_check_zero() ignores bit_sz
on BTF_KIND_ENUM and BTF_KIND_ENUM64 and calls btf_dump_get_enum_value()
directly instead of btf_dump_bitfield_check_zero():

tools/lib/bpf/btf_dump.c:btf_dump_type_data_check_zero() {
	...
	case BTF_KIND_ENUM:
	case BTF_KIND_ENUM64:
		err = btf_dump_get_enum_value(d, t, data, id, &value);
		if (err)
			return err;
	...
}

Previously, the unaligned path called btf_dump_get_bitfield_value(d, t,
data, 0, 0, &val), which computed nr_bytes = 0 and did not fail the bounds
check.  Requiring data + t->size <= d->typed_dump->data_end rejects any
unaligned enum bitfield whose underlying enum size extends past the end of
the struct, causing btf_dump__dump_type_data() to fail with -E2BIG on a
full-sized buffer.

[Severity: Medium]
This problem wasn't introduced by this patch, but can
btf_dump_type_data_check_zero() read past d->typed_dump->data_end
[1] when dumping a truncated composite type?

btf_dump_type_data_check_overflow() intentionally skips bounds checks on
BTF_KIND_STRUCT and BTF_KIND_ARRAY so truncated structures and arrays can
be partially dumped.  Before dumping each member, however,
btf_dump_type_data_check_zero() recurses into the composite type and calls
helpers such as btf_dump_base_type_check_zero() and the aligned path of
btf_dump_get_enum_value(), which dereference data without checking that
data + nr_bytes or data + t->size fits within d->typed_dump->data_end.

[1] https://sashiko.dev/#/bug/linux-c12e98ee-e737-47d5-ad42-ceec0d189844

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