[PATCH bpf-next v2 0/2] Fix unaligned enum values in btf_dump

[PATCH bpf-next v2 0/2] Fix unaligned enum values in btf_dump

[Hungyu Lin]

btf_dump_get_enum_value() reads unaligned enum values through
btf_dump_get_bitfield_value() with a zero bit size, which shifts a
__u64 by 64. Patch 1 copies unaligned enum data into an aligned buffer
instead, as btf_dump_int_data() does. Patch 2 adds the tests Alexei
posted in the v1 review.

Changes in v2:
- Describe the undefined shift in the commit log.
- Drop the data_end check, which broke the dump of struct fs_context
  without emit_zeroes.
- Add the selftests from the v1 review.

v1: https://lore.kernel.org/bpf/20261113223449.97604-1-dennylin0707@gmail.com/

Hungyu Lin (2):
  libbpf: Fix sign extension of unaligned enums in
    btf_dump_get_enum_value()
  selftests/bpf: Dump unaligned enums and a zeroed fs_context

 tools/lib/bpf/btf_dump.c                          | 13 ++++---------
 tools/testing/selftests/bpf/prog_tests/btf_dump.c | 10 +++++++++-
 2 files changed, 13 insertions(+), 10 deletions(-)


base-commit: 0021f17128de5dc0b19f1a523f448df2440d8088
-- 
2.43.0

[PATCH bpf-next v2 1/2] libbpf: Fix sign extension of unaligned enums in btf_dump_get_enum_value()

[Hungyu Lin]

btf_dump_get_enum_value() reads unaligned enum values through
btf_dump_get_bitfield_value() with a zero bit size. That makes
right_shift_bits 64 in

	*value = (num << left_shift_bits) >> right_shift_bits;

which is undefined behavior for a __u64 on both endians. The path also
bypasses the signed loads used for aligned data, so a negative value of
a signed enum can lose its enumerator name when the data is unaligned.

Fix this by copying unaligned enum data into an aligned buffer, as
btf_dump_int_data() does, and reading it with the same loads as aligned
data.

Fixes: 920d16af9b42 ("libbpf: BTF dumper support for typed data")
Signed-off-by: Hungyu Lin <dennylin0707@gmail.com>
---
 tools/lib/bpf/btf_dump.c | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/tools/lib/bpf/btf_dump.c b/tools/lib/bpf/btf_dump.c
index bbe86928b3ec..a80762a70cd0 100644
--- a/tools/lib/bpf/btf_dump.c
+++ b/tools/lib/bpf/btf_dump.c
@@ -2329,16 +2329,11 @@ 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)) {
+		memcpy(buf, data, t->size);
+		data = buf;
 	}
 
 	switch (t->size) {
-- 
2.43.0

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

[Hungyu Lin]

Dump the signed enum value from an unaligned address in the enum data
test. Also dump a zeroed struct fs_context without emit_zeroes in the
struct data test, which covers phase, an enum bitfield 3 bytes before
the end of the struct.

Suggested-by: Alexei Starovoitov <ast@kernel.org>
Signed-off-by: Hungyu Lin <dennylin0707@gmail.com>
---
 tools/testing/selftests/bpf/prog_tests/btf_dump.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/btf_dump.c b/tools/testing/selftests/bpf/prog_tests/btf_dump.c
index b5c90f729339..d6fd9933ff25 100644
--- a/tools/testing/selftests/bpf/prog_tests/btf_dump.c
+++ b/tools/testing/selftests/bpf/prog_tests/btf_dump.c
@@ -945,7 +945,7 @@ static void test_btf_dump_enum_data(struct btf *btf, struct btf_dump *d,
 				    char *str)
 {
 	__u32 high = 0x80000000;
-	__s32 neg = -1;
+	__s32 neg[2] = { -1, -1 };
 
 	/* enum where enum value does (and does not) exist */
 	TEST_BTF_DUMP_DATA_C(btf, d, "enum", str, enum bpf_cmd, BTF_F_COMPACT,
@@ -994,6 +994,8 @@ static void test_btf_dump_enum_data(struct btf *btf, struct btf_dump *d,
 				"(enum test_enum_u32)TEST_ENUM_HIGH"), "dump unsigned enum");
 	ASSERT_OK(btf_dump_data(btf, d, "test_enum_s32", NULL, 0, &neg, 4, str,
 				"(enum test_enum_s32)TEST_ENUM_NEG"), "dump signed enum");
+	ASSERT_OK(btf_dump_data(btf, d, "test_enum_s32", NULL, 0, (void *)neg + 1, 4, str,
+				"(enum test_enum_s32)TEST_ENUM_NEG"), "dump unaligned enum");
 	btf_dump__free(d);
 }
 
@@ -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");
+
+		str[0] = '\0';
 		opts.emit_zeroes = true;
 		ret = btf_dump__dump_type_data(d, type_id, zero_data, type_sz, &opts);
 		ASSERT_EQ(ret, type_sz,
-- 
2.43.0

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

[sashiko-bot]

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