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

On Fri, Mar 13, 2026 at 1:15 AM Alan Maguire <[email protected]> wrote:
>
> On 11/03/2026 23:37, Andrii Nakryiko wrote:
> > On Wed, Mar 11, 2026 at 12:35 PM Alan Maguire <[email protected]> wrote:
> >>
> >> On 11/03/2026 19:21, Andrii Nakryiko wrote:
> >>> 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"
> >>>
> >>
> >> would btf->unknown_hdr_data work?
> >
> > what about btf->has_hdr_extra ?
> >>
> >
>
> Perfect. BTW I've been exploring an approach to do libbpf sanitization code testing
> on upstream kernels. The idea is we make kernel_supports() __weak in libbpf so that
> tests can override it to simulate old kernels lacking features using test_progs
> own version of kernel_supports() in btf_helpers.c. Tests set a thread-specific key
> (so no parallel test interference) if they want to override behaviour, otherwise
> we call the real kernel_supports() in libbpf. Would give us some extra test coverage
> around sanitization (and potentially other areas which check kernel support) since
> we'll have more to do in that area for inlines. What do you think? I can separate it
> from the current series if it seems useful.

ok, that seems contained enough. Just __weak on kernel_supports()
doesn't seem like a big deal to me