Re: [PATCH v13 bpf-next 2/8] libbpf: Support layout section handling in BTF
Alan Maguire <[email protected]>
| Newsgroups | org.kernel.vger.dwarves,org.kernel.vger.bpf |
|---|---|
| Message-ID | <[email protected]> |
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). 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); > > [..]