Re: [PATCH v2 bpf-next 16/18] btf: Relocate inline BTF for modules with distilled base BTF

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Kernel panic via unconditional btf_free(NULL) on error path
- [Medium] Memory leak of vmlinux.inline sysfs attribute data across module unloads and reloads
- [Medium] Concurrent updates and reads of `vmlinux.inline` BTF sysfs data can cause an out-of-bounds read
--

commit aad19c968ab1d821749bb6d06563d7c786f0721d
Author: Alan Maguire <alan.maguire@oracle.com>

btf: Relocate inline BTF for modules with distilled base BTF

This commit adds support for parsing, relocating, and publishing inline BTF
for modules that use a distilled base BTF. It parses the module's inline
BTF relative to the original module BTF, rewrites the IDs and string
offsets, and ensures a failure to process inline BTF doesn't block the
regular module BTF registration.

> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index 2ac1f1d396608..2f3e8cea7dfcd 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
[ ... ]
> @@ -6727,15 +6727,140 @@ __u32 btf_relocate_id(const struct btf *btf, __u32 id)
[ ... ]
>  errout:
> +	kvfree(module_str_map);
>  	btf_verifier_env_free(env);
> +	btf_free(inline_btf);
>  	if (!IS_ERR(base_btf) && base_btf != vmlinux_btf)
>  		btf_free(base_btf);
> -	if (btf) {
> -		kvfree(btf->data);
> -		kvfree(btf->types);
> -		kfree(btf);
> -	}
> +	btf_free(btf);

[Severity: High]
Is it safe to unconditionally call btf_free() on these pointers here?

If btf_parse_split() returns an error, btf or inline_btf can be NULL before
jumping to errout. Looking at btf_free(), it immediately passes the pointer
to btf_free_struct_meta_tab(), which dereferences it without checking for
NULL:

static void btf_free(struct btf *btf)
{
	btf_free_struct_meta_tab(btf);
...
static void btf_free_struct_meta_tab(struct btf *btf)
{
	struct btf_struct_metas *tab = btf->struct_meta_tab;

Could this lead to a NULL pointer dereference and a kernel panic during an
error path?


>  	return ERR_PTR(err);
>  }
[ ... ]
> @@ -8958,32 +9104,26 @@ static int btf_module_notify(struct notifier_block *nb, unsigned long op,
>  		btf_mod->sysfs_attr = attr;
>  
>  #if IS_ENABLED(CONFIG_DEBUG_INFO_BTF_INLINE)
> -		if (mod->btf_inline_data_size > 0) {
> +		if (relocated_inline_data) {
>  			char name[MODULE_NAME_LEN + sizeof(".inline")];
>  
>  			if (strcmp(mod->name, "btf_vmlinux_inline") == 0) {
>  				if (vmlinux_inline_attr)
> -					sysfs_btf_update(vmlinux_inline_attr, data,
> -							 mod->btf_inline_data_size);
> +					sysfs_btf_update(vmlinux_inline_attr,
> +							 relocated_inline_data, inline_data_size);

[Severity: Medium]
Could the concurrent update of this sysfs attribute cause an out-of-bounds
read?

Looking at sysfs_btf_update(), the new pointer is written before the new
size:

static void sysfs_btf_update(struct bin_attribute *attr, void *data,
                             size_t data_size)
{
	...
	WRITE_ONCE(attr->private, data);
	smp_store_release(&attr->size, data_size);
}

If a concurrent reader in sysfs_btf_kernfs_read() picks up the old, larger
size via smp_load_acquire(), but then reads the newly updated pointer from
attr->private via READ_ONCE(), wouldn't the subsequent memcpy() read
out-of-bounds past the end of the new, smaller data buffer?


>  				else
> -					kvfree(data);
> +					kvfree(relocated_inline_data);
> +				btf_mod->btf_inline_data = NULL;

[Severity: Medium]
Does setting btf_mod->btf_inline_data to NULL here cause a memory leak when
the btf_vmlinux_inline module is unloaded?

Because it's set to NULL, the normal cleanup path in btf_module_notify()
won't free it. Then, if the module is reloaded, sysfs_btf_update() will
overwrite the vmlinux_inline_attr->private pointer with a new allocation,
leaving the old relocated_inline_data stranded.


>  				break;
>  			}
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260901165757.801449-1-alan.maguire@oracle.com?part=16