Re: [PATCH v6 bpf-next 04/10] libbpf: Add kind layout encoding support

Alan Maguire
2025-12-11T08:37:05Z

On 10/12/2025 20:55, bot+bpf-ci@kernel.org wrote:
>> diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
>> index 4eb0704a0309..2133e976cb9c 100644
>> --- a/tools/lib/bpf/btf.c
>> +++ b/tools/lib/bpf/btf.c
>
> [ ... ]
>
>> -static struct btf *btf_new_empty(struct btf *base_btf)
>> +static struct btf *btf_new_empty(struct btf_new_opts *opts)
>> {
>> + struct btf *base_btf = OPTS_GET(opts, base_btf, NULL);
>> struct btf_header *hdr;
>> struct btf *btf;
>>
>> @@ -1111,6 +1141,17 @@ static struct btf *btf_new_empty(struct btf *base_btf)
>> free(btf);
>> return ERR_PTR(-ENOMEM);
>> }
>> +
>> + if (opts->add_kind_layout) {
>
> Can opts be NULL here? The OPTS_VALID() check in btf__new_empty_opts()
> returns true when opts is NULL (!(opts) evaluates to true), so this code
> could dereference NULL. Should this use OPTS_GET(opts, add_kind_layout,
> false) instead?

Will do.

>
>> + hdr->kind_layout_len = sizeof(kind_layouts);
>> + btf->kind_layout = malloc(hdr->kind_layout_len);
>> + if (!btf->kind_layout) {
>> + free(btf->hdr);
>> + free(btf);
>> + return ERR_PTR(-ENOMEM);
>> + }
>
> Does this error path leak btf->raw_data? It was allocated earlier via
> calloc() but isn't freed here, while btf->hdr is freed. The same issue
> appears in the error path above at the check "if (!btf->hdr)" where
> btf->raw_data is also not freed.
>

Will fix.

>> + memcpy(btf->kind_layout, kind_layouts, sizeof(kind_layouts));
>> + }
>> memcpy(btf->hdr, hdr, sizeof(*hdr));
>>
>> return btf;
>
> Also, when a BTF object is created via btf_new_empty() with
> add_kind_layout=true, btf->kind_layout is allocated but btf->modifiable
> remains false (never set to true). Later, when btf__free() is called, it
> only frees btf->kind_layout if btf_is_modifiable() returns true (checking
> the modifiable flag). Does this leak the kind_layout allocation for BTF
> objects created with btf__new_empty_opts()?
>

We can free btf->kind_layout conditionally as long as it is not pointing into
the btf data section (as would be the case for parsed BTF). Will add the check
in btf__free().

>> @@ -1118,12 +1159,26 @@ static struct btf *btf_new_empty(struct btf *base_btf)
>
> [ ... ]
>
>> +struct btf *btf__new_empty_opts(struct btf_new_opts *opts)
>> +{
>> + if (!OPTS_VALID(opts, btf_new_opts))
>> + return libbpf_err_ptr(-EINVAL);
>> +
>> + return libbpf_ptr(btf_new_empty(opts));
>> +}
>
> [ ... ]
>
>
> ---
> AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
> See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
>
> CI run summary: https://github.com/kernel-patches/bpf/actions/runs/20112692486