[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