[PATCH] libbpf: fix missing minus sign in btf_dump struct zero-check

[PATCH] libbpf: fix missing minus sign in btf_dump struct zero-check

[Gu Bin]

btf_dump_type_data_check_zero() returns -ENODATA when a datum is all
zero (nothing to dump). In the BTF_KIND_STRUCT case the per-member
result is compared with ENODATA (no minus sign), unlike every other
branch which uses -ENODATA. The function only ever returns -ENODATA,
so the comparison `err != ENODATA` is always true and the loop bails
out on the first all-zero member: a struct that still has non-zero
members is reported as empty and skipped by the dumper.

Use -ENODATA so non-zero members are handled correctly.

Fixes: 920d16af9b42 ("libbpf: BTF dumper support for typed data")
Signed-off-by: Gu Bin <gubin@cmss.chinamobile.com>
---
 tools/lib/bpf/btf_dump.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/lib/bpf/btf_dump.c b/tools/lib/bpf/btf_dump.c
index 7dedd3975c39..ea9a34db4c00 100644
--- a/tools/lib/bpf/btf_dump.c
+++ b/tools/lib/bpf/btf_dump.c
@@ -2567,7 +2567,7 @@ static int btf_dump_type_data_check_zero(struct btf_dump *d,
 			bit_sz = btf_member_bitfield_size(t, i);
 			err = btf_dump_type_data_check_zero(d, mtype, m->type, data + moffset / 8,
 							    moffset % 8, bit_sz);
-			if (err != ENODATA)
+			if (err != -ENODATA)
 				return err;
 		}
 		return -ENODATA;
-- 
2.55.0.windows.3

Re: [PATCH] libbpf: fix missing minus sign in btf_dump struct zero-check

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 5319 bytes --]

> libbpf: fix missing minus sign in btf_dump struct zero-check

TL;DR:
- With the fix, btf_dump's nested struct zero-scan may read past the caller's
  data_sz buffer (possible out-of-bounds read)
- Commit message inaccurate about return values and omits the behaviour change

> libbpf: fix missing minus sign in btf_dump struct zero-check
>
> btf_dump_type_data_check_zero() returns -ENODATA when a datum is all
> zero (nothing to dump). In the BTF_KIND_STRUCT case the per-member
> result is compared with ENODATA (no minus sign), unlike every other
> branch which uses -ENODATA. The function only ever returns -ENODATA,
> so the comparison `err != ENODATA` is always true and the loop bails
> out on the first all-zero member: a struct that still has non-zero
> members is reported as empty and skipped by the dumper.
>
> Use -ENODATA so non-zero members are handled correctly.
>
> Fixes: 920d16af9b42 ("libbpf: BTF dumper support for typed data")
> Signed-off-by: Gu Bin <gubin@cmss.chinamobile.com>

The commit message says "The function only ever returns -ENODATA", but is
that accurate?

btf_dump_type_data_check_zero() returns 0 when the data is non-zero,
-ENODATA when it is all zero, and it also propagates errors such as -EINVAL
and -E2BIG from btf_dump_base_type_check_zero(),
btf_dump_get_bitfield_value() and btf_dump_get_enum_value().

What makes the comparison always true is that the function never returns the
positive value ENODATA.  Could this be reworded, for example "it returns
-ENODATA, never the positive ENODATA, so the comparison is always true"?

The message also doesn't say what changes after this patch: the zero scan
now visits all members instead of only the first one, so nested structs and
unions whose leading members are zero are now dumped when a later member is
non-zero.  Could that be mentioned?

> diff --git a/tools/lib/bpf/btf_dump.c b/tools/lib/bpf/btf_dump.c
> index 53c6624161d7..fb8fd8b28a7d 100644
> --- a/tools/lib/bpf/btf_dump.c
> +++ b/tools/lib/bpf/btf_dump.c

[ ... ]

> @@ -2468,7 +2468,7 @@ static int btf_dump_type_data_check_zero(struct btf_dump *d,
>  			bit_sz = btf_member_bitfield_size(t, i);
>  			err = btf_dump_type_data_check_zero(d, mtype, m->type, data + moffset / 8,
>  							    moffset % 8, bit_sz);
> -			if (err != ENODATA)
> +			if (err != -ENODATA)
>  				return err;
                                ^^^^

With the sign fixed, the STRUCT/UNION branch of
btf_dump_type_data_check_zero() now walks every member of a nested aggregate
until it finds a non-zero one.  Before, it returned after the first member,
so the scan never went past it.

Can this scan read past d->typed_dump->data_end?

btf_dump_base_type_check_zero() does an unconditional
memcmp(data, bytecmp, nr_bytes), and btf_dump_get_enum_value() dereferences
data with no bounds check either.

In the normal dump path a member is checked first, since
btf_dump_dump_type_data() calls btf_dump_type_data_check_overflow() for base
types and returns -E2BIG before check_zero runs.  That doesn't cover the
aggregate itself, because check_overflow() only checks INT, FLOAT, PTR and
ENUM kinds.

The library explicitly supports a data_sz smaller than the type size (see the
comment in btf_dump_type_data_check_overflow() and the TEST_BTF_DUMP_DATA_OVER
selftests), and commit 5714ca8cba5e fixed an OOB read of a similar kind in
this file.

Concretely, take:

    struct outer { int pad; struct inner { int a; int b; } in; };

and call btf_dump__dump_type_data() with data_sz = 8, so data_end = data + 8:

  1. btf_dump_dump_type_data(outer, depth 0) calls check_overflow(), which
     has no bounds check for a struct, and check_zero(), which returns 0 at
     depth 0.

  2. btf_dump_struct_data() raises depth to 1.  For member 'in' (data + 4),
     check_overflow() again does no check for a struct, then
     check_zero(inner) runs.

  3. Member 'a' (data + 4..8) is in range and zero, so it returns -ENODATA.
     With the old 'err != ENODATA' the function returned -ENODATA here and
     the struct was skipped with no further reads.  With '-ENODATA' the loop
     continues.

  4. Member 'b' is at data + 8..12, past data_end.
     btf_dump_base_type_check_zero() calls memcmp(data + 8, bytecmp, 4) and
     reads 4 bytes past the caller's buffer.

The over-read is bounded only by the size of the nested aggregate, so a
nested struct with a large array member can read much further.

The reads past data_end only happen when the leading members are zero.  If
the out-of-range bytes happen to be non-zero, the struct is dumped and the
later btf_dump_dump_type_data() call returns -E2BIG for that member.  If they
are zero, the struct is silently skipped and the dump reports success.

The BTF_KIND_ARRAY branch of the same function has a similar unbounded scan,
but it is not changed by this patch, whereas the struct/union scan past the
first member is newly reachable.

Should the zero scan be bounded by d->typed_dump->data_end in the
struct/array branches, or should check_overflow() run on each member before
the check_zero() recursion?


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/41417286408

[PATCH v2] libbpf: Fix ENODATA comparison and unbounded zero-scan in btf_dump

[Gu Bin]

In btf_dump_type_data_check_zero(), the BTF_KIND_STRUCT/UNION branch
compares the per-member result with ENODATA (no minus sign), unlike
every other branch which uses -ENODATA.  The function returns
-ENODATA, 0, or propagated errors such as -EINVAL and -E2BIG; it never
returns the positive ENODATA, so the comparison is always true and the
loop bails out on the first member: a struct whose later members are
non-zero is reported as empty and skipped by the dumper.

Separately, the zero-scan loops over struct/union members and array
elements do not check for overflow: btf_dump_type_data_check_overflow()
only validates base types, while the scan recurses into aggregates and
reads member data without bounds checks.  The library explicitly
supports a data_sz smaller than the type size, so the scan can read
past the caller's buffer, similar to the OOB read fixed by commit
5714ca8cba5e ("libbpf: Fix OOB read in btf_dump_get_bitfield_value").

Fix the comparison to use -ENODATA, so the zero-scan visits every
member and a nested struct or union whose leading members are zero is
dumped when a later member is non-zero.  Also bound the zero-scan:
call btf_dump_type_data_check_overflow() on each struct/union member
and array element before the check_zero() recursion, mirroring what
the dump path does before reading a value.

Fixes: 920d16af9b42 ("libbpf: BTF dumper support for typed data")
Signed-off-by: Gu Bin <gubin@cmss.chinamobile.com>
---
Changes in v2 (addressing the bpf-ci review of v1):

- v1's commit message claimed check_zero() "only ever returns
  -ENODATA"; it actually returns 0, -ENODATA, or propagated errors
  (-EINVAL, -E2BIG), and never the positive ENODATA, which is what
  makes the comparison always true.  Reworded.

- v1 fixed only the sign.  That made the struct/union zero-scan walk
  past the first member for the first time, making an existing
  unbounded-scan OOB reachable on that path (the array branch already
  had the same unbounded scan, it just was not triggered by the sign
  bug).  v2 bounds both loops by running check_overflow() on each
  member/element before the check_zero() recursion.

- The message now also states the behaviour change: nested
  structs/unions whose leading members are zero are no longer
  reported as empty when a later member is non-zero.
---
 tools/lib/bpf/btf_dump.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/tools/lib/bpf/btf_dump.c b/tools/lib/bpf/btf_dump.c
index 7dedd3975c39..113d89ef1cd9 100644
--- a/tools/lib/bpf/btf_dump.c
+++ b/tools/lib/bpf/btf_dump.c
@@ -2533,6 +2533,12 @@ static int btf_dump_type_data_check_zero(struct btf_dump *d,
 		 * non-zero because the string is terminated.
 		 */
 		for (i = 0; i < array->nelems; i++) {
+			/* Check overflow first, as the dump path does. */
+			err = btf_dump_type_data_check_overflow(d, elem_type,
+				elem_type_id, data + (i * elem_size),
+				bits_offset, 0);
+			if (err < 0)
+				return err;
 			if (i == 0 && ischar && *(char *)data == 0)
 				return -ENODATA;
 			err = btf_dump_type_data_check_zero(d, elem_type,
@@ -2565,9 +2571,15 @@ static int btf_dump_type_data_check_zero(struct btf_dump *d,
 			 * of member can retrieve it.
 			 */
 			bit_sz = btf_member_bitfield_size(t, i);
+			/* Check overflow first, as the dump path does. */
+			err = btf_dump_type_data_check_overflow(d, mtype, m->type,
+							       data + moffset / 8,
+							       moffset % 8, bit_sz);
+			if (err < 0)
+				return err;
 			err = btf_dump_type_data_check_zero(d, mtype, m->type, data + moffset / 8,
 							    moffset % 8, bit_sz);
-			if (err != ENODATA)
+			if (err != -ENODATA)
 				return err;
 		}
 		return -ENODATA;
-- 
2.55.0.windows.3

Re: [PATCH v2] libbpf: Fix ENODATA comparison and unbounded zero-scan in btf_dump

[Alexei Starovoitov]

On Mon, Nov 02, 2026 at 05:16 AM Gu Bin <gubin@cmss.chinamobile.com> wrote:
> In btf_dump_type_data_check_zero(), the BTF_KIND_STRUCT/UNION branch
> compares the per-member result with ENODATA (no minus sign), unlike
> every other branch which uses -ENODATA.  The function returns

This part is already fixed in bpf-next. See
commit ab332ec8953d ("libbpf: Fix struct and union zero detection in BTF dumper").
Pls rebase. Drop this part from the subject and from the commit log.

[...]

> @@ -2533,6 +2533,12 @@ static int btf_dump_type_data_check_zero(struct btf_dump *d,
> +			err = btf_dump_type_data_check_overflow(d, elem_type,
> @@ -2565,9 +2571,15 @@ static int btf_dump_type_data_check_zero(struct btf_dump *d,
> +			err = btf_dump_type_data_check_overflow(d, mtype, m->type,

No need for two copies. Do the check once at the top of
btf_dump_type_data_check_zero(). Every member and every element goes
through it.
The only other read is *(char *)data in the array branch. The recursion
for element 0 gives the same answer, so that line can go.

Something like this (not even compile tested):

diff --git a/tools/lib/bpf/btf_dump.c b/tools/lib/bpf/btf_dump.c
index ea9a34d..d842ae1 100644
--- a/tools/lib/bpf/btf_dump.c
+++ b/tools/lib/bpf/btf_dump.c
@@ -2504,6 +2504,11 @@ static int btf_dump_type_data_check_zero(struct btf_dump *d,
 	     !d->typed_dump->is_array_char))
 		return 0;

+	/* members and elements get here without a check against data_end */
+	err = btf_dump_type_data_check_overflow(d, t, id, data, bits_offset, bit_sz);
+	if (err < 0)
+		return err;
+
 	t = skip_mods_and_typedefs(d->btf, id, NULL);

 	switch (btf_kind(t)) {
@@ -2533,14 +2538,12 @@ static int btf_dump_type_data_check_zero(struct btf_dump *d,
 		 * non-zero because the string is terminated.
 		 */
 		for (i = 0; i < array->nelems; i++) {
-			if (i == 0 && ischar && *(char *)data == 0)
-				return -ENODATA;
 			err = btf_dump_type_data_check_zero(d, elem_type,
 							    elem_type_id,
 							    data +
 							    (i * elem_size),
 							    bits_offset, 0);
-			if (err != -ENODATA)
+			if (err != -ENODATA || ischar)
 				return err;
 		}
 		return -ENODATA;

Pls add a selftest as patch 2. TEST_BTF_DUMP_DATA_OVER() with an all-zero
union bpf_iter_link_info and sizeof() - 1 is enough.
bpf-next returns 16 for it today. It has to return -E2BIG.

Pls tag the respin as [PATCH bpf-next v3].

pw-bot: cr

[PATCH bpf-next v3 0/2] Fix OOB read in BTF dumper zero-scan

[Gu Bin]

v2 fixed two issues: the missing minus sign in the struct/union
ENODATA comparison, and the unbounded zero-scan.  As Alexei pointed
out, the sign fix has since landed as commit ab332ec8953d ("libbpf:
Fix struct and union zero detection in BTF dumper"), so this respin
is rebased on top of it and covers the OOB only.

The overflow check now runs once at the top of
btf_dump_type_data_check_zero(), as suggested, instead of in both
member/element loops, and the direct *(char *)data == 0 test in the
array branch is dropped in favour of the bounds-checked element-0
recursion.

Patch 2 adds the requested selftest: an all-zero
union bpf_iter_link_info with sizeof() - 1 must return -E2BIG.

Gu Bin (2):
  libbpf: Fix OOB read in BTF dumper zero-scan
  selftests/bpf: Add overflow test for BTF dump zero-scan

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

-- 
2.55.0.windows.3

[PATCH bpf-next v3 1/2] libbpf: Fix OOB read in BTF dumper zero-scan

[Gu Bin]

btf_dump_type_data_check_zero() recurses into struct/union members and
array elements without checking the data against
d->typed_dump->data_end.  The dump path performs an overflow check
before reading a value, but that check does not apply to the zero-scan
recursion, so a caller passing a data_sz smaller than the type size
gets reads past the end of the buffer instead of the -E2BIG the API
promises, similar to the OOB read fixed by commit 5714ca8cba5e
("libbpf: Fix OOB read in btf_dump_get_bitfield_value").

Check for overflow in btf_dump_type_data_check_zero(), after the
toplevel exceptions: every member and element recursion passes through
it, mirroring what the dump path does before reading a value.

Also drop the direct *(char *)data == 0 test in the array branch; the
recursion for element 0 gives the same answer and is bounds-checked
now.

Fixes: 920d16af9b42 ("libbpf: BTF dumper support for typed data")
Suggested-by: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Signed-off-by: Gu Bin <gubin@cmss.chinamobile.com>
---
 tools/lib/bpf/btf_dump.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/tools/lib/bpf/btf_dump.c b/tools/lib/bpf/btf_dump.c
index ea9a34db4c00..d842ae14ef5c 100644
--- a/tools/lib/bpf/btf_dump.c
+++ b/tools/lib/bpf/btf_dump.c
@@ -2504,6 +2504,11 @@ static int btf_dump_type_data_check_zero(struct btf_dump *d,
 	     !d->typed_dump->is_array_char))
 		return 0;
 
+	/* members and elements get here without a check against data_end */
+	err = btf_dump_type_data_check_overflow(d, t, id, data, bits_offset, bit_sz);
+	if (err < 0)
+		return err;
+
 	t = skip_mods_and_typedefs(d->btf, id, NULL);
 
 	switch (btf_kind(t)) {
@@ -2533,14 +2538,12 @@ static int btf_dump_type_data_check_zero(struct btf_dump *d,
 		 * non-zero because the string is terminated.
 		 */
 		for (i = 0; i < array->nelems; i++) {
-			if (i == 0 && ischar && *(char *)data == 0)
-				return -ENODATA;
 			err = btf_dump_type_data_check_zero(d, elem_type,
 							    elem_type_id,
 							    data +
 							    (i * elem_size),
 							    bits_offset, 0);
-			if (err != -ENODATA)
+			if (err != -ENODATA || ischar)
 				return err;
 		}
 		return -ENODATA;
-- 
2.55.0.windows.3

[PATCH bpf-next v3 2/2] selftests/bpf: Add overflow test for BTF dump zero-scan

[Gu Bin]

Add a TEST_BTF_DUMP_DATA_OVER() case that dumps an all-zero
union bpf_iter_link_info with a data_sz one byte short of the type
size.  Without the overflow check in
btf_dump_type_data_check_zero() the zero-scan reads past
data_end; with it, btf_dump__dump_type_data() returns -E2BIG.

Suggested-by: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Signed-off-by: Gu Bin <gubin@cmss.chinamobile.com>
---
 tools/testing/selftests/bpf/prog_tests/btf_dump.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/tools/testing/selftests/bpf/prog_tests/btf_dump.c b/tools/testing/selftests/bpf/prog_tests/btf_dump.c
index dbef014f75ac..f6cff6cc56c4 100644
--- a/tools/testing/selftests/bpf/prog_tests/btf_dump.c
+++ b/tools/testing/selftests/bpf/prog_tests/btf_dump.c
@@ -1161,6 +1161,13 @@ static void test_btf_dump_struct_data(struct btf *btf, struct btf_dump *d,
 			   "(union bpf_iter_link_info){.cgroup = (struct){.cgroup_fd = (__u32)1,},.task = (struct){.pid = (__u32)1,},}",
 			   { .cgroup = { .cgroup_fd = 1 }});
 
+	/* overflow test for all-zero union: with a data_sz one byte short
+	 * of the type size, the zero-scan must not read past data_end and
+	 * must return -E2BIG.
+	 */
+	TEST_BTF_DUMP_DATA_OVER(btf, d, "union", str, union bpf_iter_link_info,
+				sizeof(union bpf_iter_link_info) - 1, "", {});
+
 	/* struct skb with nested structs/unions; because type output is so
 	 * complex, we don't do a string comparison, just verify we return
 	 * the type size as the amount of data displayed.
-- 
2.55.0.windows.3

Re: [PATCH bpf-next v3 2/2] selftests/bpf: Add overflow test for BTF dump zero-scan

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] selftests/bpf: wrong expected_val in test_btf_dump_struct_data() overflow test
- [Low] selftests/bpf: multi-line comment style in test_btf_dump_struct_data()
--

commit c037995f7dcc94c6cb7bb8863c0b417b37425882
Author: Gu Bin <gubin@cmss.chinamobile.com>

selftests/bpf: Add overflow test for BTF dump zero-scan

This commit adds a TEST_BTF_DUMP_DATA_OVER() test case in
test_btf_dump_struct_data() that dumps an all-zero union bpf_iter_link_info
with a data_sz one byte short of the type size to verify that the zero-scan
returns -E2BIG instead of reading past data_end.

> diff --git a/tools/testing/selftests/bpf/prog_tests/btf_dump.c b/tools/testing/selftests/bpf/prog_tests/btf_dump.c
> index dbef014f75acd..f6cff6cc56c4f 100644
> --- a/tools/testing/selftests/bpf/prog_tests/btf_dump.c
> +++ b/tools/testing/selftests/bpf/prog_tests/btf_dump.c
> @@ -1161,6 +1161,13 @@ static void test_btf_dump_struct_data(struct btf *btf, struct btf_dump *d,
>  			   "(union bpf_iter_link_info){.cgroup = (struct){.cgroup_fd = (__u32)1,},.task = (struct){.pid = (__u32)1,},}",
>  			   { .cgroup = { .cgroup_fd = 1 }});
>  
> +	/* overflow test for all-zero union: with a data_sz one byte short
> +	 * of the type size, the zero-scan must not read past data_end and
> +	 * must return -E2BIG.
> +	 */

[Severity: Low]
This isn't a bug, but in BPF code multi-line comments place the opening /*
on its own line with the comment text starting on the next line. Could this
comment be formatted to match the BPF style?

> +	TEST_BTF_DUMP_DATA_OVER(btf, d, "union", str, union bpf_iter_link_info,
> +				sizeof(union bpf_iter_link_info) - 1, "", {});

[Severity: Medium]
Will this test fail the expected string check in btf_dump_data()?

When btf_dump__dump_type_data() enters btf_dump_dump_type_data() at
d->typed_dump->depth == 0 for a union:

tools/lib/bpf/btf_dump.c:btf_dump_dump_type_data() {
	size = btf_dump_type_data_check_overflow(d, t, id, data, bits_offset, bit_sz);
	if (size < 0)
		return size;
	err = btf_dump_type_data_check_zero(d, t, id, data, bits_offset, bit_sz);
	if (err) {
		...
	}
	btf_dump_data_pfx(d);

	if (!d->typed_dump->skip_names) {
		if (fname && strlen(fname) > 0)
			btf_dump_printf(d, ".%s = ", fname);
		btf_dump_emit_type_cast(d, id, true);
	}
}

At depth == 0, btf_dump_type_data_check_overflow() only checks base types and
returns the union size (16), and btf_dump_type_data_check_zero() immediately
returns 0. Then btf_dump_emit_type_cast() and btf_dump_struct_data() emit
"(union bpf_iter_link_info){\n" into str before iterating over the union
members at depth == 1, where btf_dump_type_data_check_zero() on the .cgroup
member detects the overflow and returns -E2BIG.

In btf_dump_data(), after verifying that btf_dump__dump_type_data() returns
-E2BIG, it checks:

	if (!ASSERT_STREQ(str, expected_val, "ensure expected/actual match"))
		return -EFAULT;

Because str contains "(union bpf_iter_link_info){\n" while expected_val is
passed as "", shouldn't expected_val be "(union bpf_iter_link_info){\n" here
(similar to the atomic_t overflow test in test_btf_dump_int_data())?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261103032619.1378-1-gubin@cmss.chinamobile.com?part=2

Re: [PATCH bpf-next v3 2/2] selftests/bpf: Add overflow test for BTF dump zero-scan

[Alexei Starovoitov]

On Mon, Nov 02, 2026 at 07:26 PM Gu Bin <gubin@cmss.chinamobile.com> wrote:
> +	TEST_BTF_DUMP_DATA_OVER(btf, d, "union", str, union bpf_iter_link_info,
> +				sizeof(union bpf_iter_link_info) - 1, "", {});

sashiko is right. This test fails.

Pls run the test before sending v4.

pw-bot: cr