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

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

On 11/12/2025 08:36, Alan Maguire wrote:
> 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().
>

Actually the right answer here is for btf_new_empty() to place the kind layout
data in the raw contiguous representation when add_kind_layout is true; it then
becomes btf_ensure_modifiable()s job to break out and allocate sections as is true today
for type/string sections. With this approach, we do not need to allocate btf->kind_layout
in btf_new_empty() and the leak will not happen as far as I can see since allocation
will only happen when btf->modifiable is true.

>>> @@ -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
> 
>