Re: [PATCH bpf-next v2 2/2] selftests/bpf: Dump unaligned enums and a zeroed fs_context

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] selftests/bpf: enum bitfield OOB read in
  btf_dump_type_data_check_zero()
--

commit 796295518e9df1f4059137fab9435ca1e6bd6793
Author: Hungyu Lin <dennylin0707@gmail.com>

selftests/bpf: Dump unaligned enums and a zeroed fs_context

This commit extends the BTF data dump selftests to dump a signed enum from
an unaligned address and a zeroed struct fs_context without emit_zeroes.

> diff --git a/tools/testing/selftests/bpf/prog_tests/btf_dump.c b/tools/testing/selftests/bpf/prog_tests/btf_dump.c
> index b5c90f7293398..d6fd9933ff258 100644
> --- a/tools/testing/selftests/bpf/prog_tests/btf_dump.c
> +++ b/tools/testing/selftests/bpf/prog_tests/btf_dump.c
[ ... ]
> @@ -1182,6 +1184,12 @@ static void test_btf_dump_struct_data(struct btf *btf, struct btf_dump *d,
>  		type_sz = btf__resolve_size(btf, type_id);
>  		str[0] = '\0';
>  
> +		/* 'phase' is an enum bitfield 3 bytes before the end */
> +		ret = btf_dump__dump_type_data(d, type_id, zero_data, type_sz, &opts);
> +		ASSERT_EQ(ret, type_sz,
> +			  "unexpected return value dumping zeroed fs_context");

[Severity: Medium]
Does dumping struct fs_context with emit_zeroes set to false read 1 byte
past zero_data + type_sz when checking the phase enum bitfield?

In struct fs_context, phase is an 8-bit enum bitfield (t->size == 4) placed
3 bytes before the end of the struct.  btf_dump_type_data_check_overflow()
checks bit_sz (1 byte) and allows the access, but
btf_dump_type_data_check_zero() ignores bit_sz for BTF_KIND_ENUM and
BTF_KIND_ENUM64 and reads the full t->size via btf_dump_get_enum_value():

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

Because data at type_sz - 3 is unaligned for a 4-byte enum,
btf_dump_get_enum_value() copies 4 bytes with memcpy(buf, data, t->size),
reading 1 byte past d->typed_dump->data_end and mixing adjacent bitfields
and out-of-bounds memory into the zero check.  In this test, passing the
oversized 512-byte zero_data buffer and only asserting ret == type_sz hides
the out-of-bounds read from AddressSanitizer.

Should btf_dump_type_data_check_zero() call btf_dump_bitfield_check_zero()
when bit_sz is non-zero for BTF_KIND_ENUM and BTF_KIND_ENUM64, matching
BTF_KIND_INT and btf_dump_dump_type_data()?

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