Re: [PATCH v8 bpf-next 02/10] libbpf: Support kind layout section handling in BTF

[Alan Maguire <alan.maguire@oracle.com>]

On 19/12/2025 17:58, Andrii Nakryiko wrote:
> On Fri, Dec 19, 2025 at 5:34 AM Alan Maguire <alan.maguire@oracle.com> wrote:
>>
>> On 16/12/2025 19:34, Andrii Nakryiko wrote:
>>> On Mon, Dec 15, 2025 at 1:18 AM Alan Maguire <alan.maguire@oracle.com> wrote:
>>>>
>>>> Support reading in kind layout fixing endian issues on reading;
>>>> also support writing kind layout section to raw BTF object.
>>>> There is not yet an API to populate the kind layout with meaningful
>>>> information.
>>>>
>>>> As part of this, we need to consider multiple valid BTF header
>>>> sizes; the original or the kind layout-extended headers.
>>>> So to support this, the "struct btf" representation is modified
>>>> to always allocate a "struct btf_header" and copy the valid
>>>> portion from the raw data to it; this means we can always safely
>>>> check fields like btf->hdr->kind_layout_len.
>>>>
>>>> Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
>>>> ---
>>>>  tools/lib/bpf/btf.c | 260 +++++++++++++++++++++++++++++++-------------
>>>>  1 file changed, 183 insertions(+), 77 deletions(-)
>>>>
>>>> diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
>>>> index b136572e889a..8835aee6ee84 100644
>>>> --- a/tools/lib/bpf/btf.c
>>>> +++ b/tools/lib/bpf/btf.c
>>>> @@ -40,42 +40,53 @@ struct btf {
>>>>
>>>>         /*
>>>>          * When BTF is loaded from an ELF or raw memory it is stored
>>>> -        * in a contiguous memory block. The hdr, type_data, and, strs_data
>>>> +        * in a contiguous memory block. The  type_data, and, strs_data
>>>
>>> nit: two spaces, and so many commas around and ;) let's leave Oxford
>>> comma, but comma after and is weird
>>>
>>>>          * point inside that memory region to their respective parts of BTF
>>>>          * representation:
>>>>          *
>>>> -        * +--------------------------------+
>>>> -        * |  Header  |  Types  |  Strings  |
>>>> -        * +--------------------------------+
>>>> -        * ^          ^         ^
>>>> -        * |          |         |
>>>> -        * hdr        |         |
>>>> -        * types_data-+         |
>>>> -        * strs_data------------+
>>>> +        * +--------------------------------+---------------------+
>>>> +        * |  Header  |  Types  |  Strings  |Optional kind layout |
>>>
>>> Space missing, boo. Keep diagrams beautiful!..
>>>
>>>> +        * +--------------------------------+---------------------+
>>>> +        * ^          ^         ^           ^
>>>> +        * |          |         |           |
>>>> +        * raw_data   |         |           |
>>>> +        * types_data-+         |           |
>>>> +        * strs_data------------+           |
>>>> +        * kind_layout----------------------+
>>>> +        *
>>>> +        * A separate struct btf_header is allocated for btf->hdr,
>>>> +        * and header information is copied into it.  This allows us
>>>> +        * to handle header data for various header formats; the original,
>>>> +        * the extended header with kind layout, etc.
>>>>          *
>>>>          * If BTF data is later modified, e.g., due to types added or
>>>>          * removed, BTF deduplication performed, etc, this contiguous
>>>> -        * representation is broken up into three independently allocated
>>>> -        * memory regions to be able to modify them independently.
>>>> +        * representation is broken up into four independent memory
>>>> +        * regions.
>>>> +        *
>>>>          * raw_data is nulled out at that point, but can be later allocated
>>>>          * and cached again if user calls btf__raw_data(), at which point
>>>> -        * raw_data will contain a contiguous copy of header, types, and
>>>> -        * strings:
>>>> +        * raw_data will contain a contiguous copy of header, types, strings
>>>> +        * and optionally kind_layout.  kind_layout optionally points to a
>>>> +        * kind_layout array - this allows us to encode information about
>>>> +        * the kinds known at encoding time.  If kind_layout is NULL no
>>>> +        * kind information is encoded.
>>>>          *
>>>> -        * +----------+  +---------+  +-----------+
>>>> -        * |  Header  |  |  Types  |  |  Strings  |
>>>> -        * +----------+  +---------+  +-----------+
>>>> -        * ^             ^            ^
>>>> -        * |             |            |
>>>> -        * hdr           |            |
>>>> -        * types_data----+            |
>>>> -        * strset__data(strs_set)-----+
>>>> +        * +----------+  +---------+  +-----------+   +-----------+
>>>> +        * |  Header  |  |  Types  |  |  Strings  |   |kind_layout|
>>>> +        * +----------+  +---------+  +-----------+   +-----------+
>>>
>>> nit: spaces (and if we go with "layout" naming, this will be short and
>>> beautiful " Layout " ;)
>>>
>>>> +        * ^             ^            ^               ^
>>>> +        * |             |            |               |
>>>> +        * hdr           |            |               |
>>>> +        * types_data----+            |               |
>>>> +        * strset__data(strs_set)-----+               |
>>>> +        * kind_layout--------------------------------+
>>>
>>> [...]
>>>
>>>> @@ -3888,7 +3989,7 @@ static int btf_dedup_strings(struct btf_dedup *d)
>>>>
>>>>         /* replace BTF string data and hash with deduped ones */
>>>>         strset__free(d->btf->strs_set);
>>>> -       d->btf->hdr->str_len = strset__data_size(d->strs_set);
>>>> +       btf_hdr_update_str_len(d->btf, strset__data_size(d->strs_set));
>>>>         d->btf->strs_set = d->strs_set;
>>>>         d->strs_set = NULL;
>>>>         d->btf->strs_deduped = true;
>>>> @@ -5343,6 +5444,11 @@ static int btf_dedup_compact_types(struct btf_dedup *d)
>>>>         d->btf->type_offs = new_offs;
>>>>         d->btf->hdr->str_off = d->btf->hdr->type_len;
>>>>         d->btf->raw_size = d->btf->hdr->hdr_len + d->btf->hdr->type_len + d->btf->hdr->str_len;
>>>> +       if (d->btf->kind_layout) {
>>>> +               d->btf->hdr->kind_layout_off = d->btf->hdr->str_off + roundup(d->btf->hdr->str_len,
>>>> +                                                                             4);
>>>> +               d->btf->raw_size = roundup(d->btf->raw_size, 4) + d->btf->hdr->kind_layout_len;
>>>
>>> maybe put layout data after type data, but before strings? rounding up
>>> string section which is byte-based feels weird. I think old libbpf
>>> implementations should handle all this well, because btf_header
>>> explicitly specifies string section offset, no?
>>>
>>
>> That sounds good, but I think there are some strictness issues with how we parse
>> BTF on the kernel side that we may need to think about, especially if we want to
>> make kind layout always available. In that case we'd need to think how old kernels
>> built with newer pahole might handle newer headers with layout info.
>>
>> First in btf_parse_hdr() the kernel rejects BTF with non-zero unsupported fields.
>> So trying to load vmlinux BTF generated by a pahole that adds layout info will
>> fail for such a kernel.
>>
>> Second when validating section info in btf_check_sec_info() we check for overlaps
>> between known sections, and we also check for gaps between known sections. Finally we
>> also check for any additional data other than the known section data.
> 
> I thought we don't validate gaps, I missed btf_check_sec_info()
> checks, though. Good for kernel, it should be strict.
> 
> But it's easy to drop this layout info in libbpf for BTF sanitization,
> this shouldn't be a problem. Just shift everything to the left and
> adjust strs_off.
> 
>>
>> For layout info stored between type+strings we'd wind up rejecting it for a few reasons:
>>
>> 1. we'd find non-zero data in the header (layout offset/len)
>> 2. we'd find a "gap" between types+strings (the layout data)
>>
>> Similarly with layout at the end
>>
>> 1. we'd find non-zero data in the header (kind layout offset/len)
>> 2. we'd find unaccounted-for data after the string data (the kind layout data)
>>
>> So either way we'd wind up with unsupported headers. One approach would be to
>> do stable backports relaxing these header tests; I think we could relax them to
>> simply ensure no overlap between sections and that sections don't overrun data
>> length without risking having unusable BTF. Then a newer BTF header with additional
>> layout info wouldn't get rejected. What do you think?
> 
> See above, I don't see why we can't just sanitize BTF and drop layout
> parts altogether. They are optional for kernel either way, no harm in
> dropping them.
>

The sanitization for user-space consumption is doable alright, I was thinking
of the case where the kernel itself reads in BTF for vmlinux/modules on boot,
and that BTF was generated by newer pahole so has unexpected layout info. 
If we just emitted layout info unconditionally that would mean newer pahole might
generate BTf for a kernel that it could not read. If however we relaxed the
constraints a bit I think we could get the validation to succeed for older
kernels while ignoring the bits of the BTF they don't care about. Fix that would
also potentially future-proof addition of other sections to the BTF header without
requiring options.

Alan