Re: [PATCH v13 bpf-next 2/8] libbpf: Support layout section handling in BTF

On Wed, Mar 11, 2026 at 12:17 PM Alan Maguire <[email protected]> wrote:
>
> On 11/03/2026 18:16, Andrii Nakryiko wrote:
> > On Wed, Mar 11, 2026 at 11:10 AM Alan Maguire <[email protected]> wrote:
> >>
> >> On 11/03/2026 17:46, Andrii Nakryiko wrote:
> >>> On Fri, Mar 6, 2026 at 3:37 AM Alan Maguire <[email protected]> wrote:
> >>>>
> >>>> Support reading in layout fixing endian issues on reading;
> >>>> also support writing layout section to raw BTF object.
> >>>> There is not yet an API to populate the layout with meaningful
> >>>> information.
> >>>>
> >>>> As part of this, we need to consider multiple valid BTF header
> >>>> sizes; the original or the layout-extended headers.
> >>>> So to support this, the "struct btf" representation is modified
> >>>> to contain a "struct btf_header" and we copy the valid
> >>>> portion from the raw data to it; this means we can always safely
> >>>> check fields like btf->hdr.layout_len
> >>>>
> >>>> Signed-off-by: Alan Maguire <[email protected]>
> >>>> ---
> >>>>  tools/lib/bpf/btf.c | 309 ++++++++++++++++++++++++++++----------------
> >>>>  1 file changed, 200 insertions(+), 109 deletions(-)
> >>>>
> >>>
> >>> [...]
> >>>
> >>>>
> >>>> -       memcpy(p, hdr, hdr->hdr_len);
> >>>> +       memcpy(p, hdr, min((__u32)sizeof(struct btf_header), hdr->hdr_len));
> >>>
> >>> in all these cases where we have sizeof(struct btf_header), this
> >>> smells like a potential issue, tbh. We are silently truncating the
> >>> original header here, potentially. Which is ok for read-only case, but
> >>> do we error out if someone is trying to modify/write such BTF where we
> >>> don't really understand all of it? Should we?
> >>>
> >>
> >> The kernel enforces that any additional data in the header beyond fields
> >> we know how to deal with is zero. We don't enforce this here, but we
> >> get a similar effect since we only copy the fields we care about. In effect
> >> we wind up removing the aspects of the BTF we don't know about, so if we
> >> read in such BTF, writing it out would not preserve any additional header
> >> information (say a new section offset/length) beyond the current size of
> >> struct btf_header. So writing such BTF after reading it in converts it to
> >> the BTF representation libbpf knows about (header fields + sections).
> >
> > I don't think we should be doing this. Let's error out when someone
> > tries to modify BTF where we don't understand *all* aspects of it.
> >
>
> ok, so to do this I think we'd need to set a never_modifiable flag or similar
> on parsing when we find non-zero unknown header data, and then fail
> btf_ensure_modifiable().

yep, I'd like some better name that would mean "there are extra btf
bits we don't understand", but I can't come up with a better name :)
if you can, prefer that over not very semantically meaningful "never
modifiable"

>
> > And also you hinted at another missing part: BTF layout info
> > "sanitization". Check bpf_object__sanitize_btf(), I think that one has
> > to drop layout information if kernel doesn't support it (unless kernel
> > will ignore layout info).
> >
> > Either way, can you please manually check that BPF object having BTF
> > with kind layout can be successfully loaded on old kernel that doesn't
> > yet know about BTF layout?
> >
>
> Good catch, we'll need to do this sanitization since older kernels will
> reject layout info in the header for program BTF that contains layout info.
> I'll add a patch to the series to handle this (not needed today but good
> to have if/when program BTF adds layout info).

yep, thanks

>
> >>
> >> That seems ok to me, but I guess the user might want to at least know this
> >> was happening; would a warning or similar when encountering additional
> >> non-zero fields be helpful maybe?
> >>
> >>
> >>
> >>>>         if (swap_endian)
> >>>> -               btf_bswap_hdr(p);
> >>>> +               btf_bswap_hdr(p, hdr->hdr_len);
> >>>>         p += hdr->hdr_len;
> >>>>
> >>>>         memcpy(p, btf->types_data, hdr->type_len);
> >>>
> >>> [..]
> >>
>