> libbpf: Fix struct and union zero detection in BTF dumper TL;DR: - Fix may read past the buffer end when dumping a truncated struct, since later members are now scanned unbounded - Missing Fixes: tag
> libbpf: Fix struct and union zero detection in BTF dumper > > btf_dump_type_data_check_zero() uses -ENODATA to indicate that a value > is zero. The array path handles this sentinel correctly, but the struct > and union path compares the return value against positive ENODATA. > > As a result, the member loop returns after examining its first member. > A nested struct or union whose first member is zero can therefore be > treated as entirely zero even when a later member is non-zero. > > Compare against -ENODATA so zero members are skipped while looking for > a non-zero member. > > Signed-off-by: Luis Vieira <[email protected]> This looks like a bug fix, but there is no Fixes: tag. The faulty "if (err != ENODATA)" comparison came in with the original typed data dumper support. Should this include: Fixes: 920d16af9b42 ("libbpf: BTF dumper support for typed data") > diff --git a/tools/lib/bpf/btf_dump.c b/tools/lib/bpf/btf_dump.c > --- a/tools/lib/bpf/btf_dump.c > +++ b/tools/lib/bpf/btf_dump.c [ ... ] > @@ -2564,7 +2564,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; Can this now read past the end of the caller's buffer when data_sz is smaller than the type? btf_dump_type_data_check_overflow() only bounds-checks base types and skips structs, unions and arrays, so a truncated struct still reaches btf_dump_type_data_check_zero(). Before this change the member loop returned after the first member. With -ENODATA, the loop proceeds to later members, and the base type checks they reach never compare against d->typed_dump->data_end: btf_dump_base_type_check_zero() does memcmp(data, bytecmp, nr_bytes) btf_dump_get_enum_value() dereferences data directly For example, with: struct outer { int a; struct inner { int x; int y; } s; }; and btf_dump__dump_type_data(d, outer_id, buf, 8, &opts) using an 8 byte buf, a != 0 and s.x == 0: btf_dump_struct_data() btf_dump_dump_type_data(s) btf_dump_type_data_check_overflow(s) /* no bounds check for STRUCT */ btf_dump_type_data_check_zero(s) x is zero, returns -ENODATA, loop continues y: btf_dump_base_type_check_zero() reads 4 bytes at data + 8, which is data_end The outcome then depends on bytes beyond the buffer. If they are non-zero, the struct is treated as non-zero and the later y member fails with -E2BIG. If they are zero, the struct is skipped and the call succeeds with the truncated data. The array branch has the same unbounded scan, but the comment on TEST_BTF_DUMP_DATA_OVER says overflow checking should trump "is field zero?" checks. Should the member checks in this path be bounded by data_end? I don't see a later patch in this series that addresses this. The follow-up selftest commit (f1b0479c0af4) only exercises a full-size buffer. --- 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/36812677691
