Re: [PATCH v13 bpf-next 2/8] libbpf: Support layout section handling in BTF
Andrii Nakryiko <[email protected]>
| Newsgroups | org.kernel.vger.dwarves,org.kernel.vger.bpf |
|---|---|
| Message-ID | <CAEf4BzZefR6qQwu1jENEMg15UCXXnfqfwHYRV5y4NAXWwuo4zw@mail.gmail.com> |
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. 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? > > 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); > > > > [..] >