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

[bot+bpf-ci@kernel.org]

[-- Attachment #1: Type: text/plain, Size: 5243 bytes --]

> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index 2ac1f1d39660..2f3e8cea7dfc 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)
>
>  #ifdef CONFIG_DEBUG_INFO_BTF_MODULES
>
> +static struct btf *btf_parse_split(struct btf_verifier_env *env, const char *name,
> +				   const void *data, unsigned int data_size,
> +				   struct btf *base_btf)
> +{
> +	struct btf *btf;
> +	int err;
> +
> +	btf = kzalloc_obj(*btf, GFP_KERNEL | __GFP_NOWARN);
> +	if (!btf)
> +		return ERR_PTR(-ENOMEM);
> +	env->btf = btf;
> +
> +	btf_set_base_btf(btf, base_btf);
> +	btf->kernel_btf = true;
> +	btf->named_start_id = 0;
> +	strscpy(btf->name, name);
> +
> +	btf->data = kvmemdup(data, data_size, GFP_KERNEL | __GFP_NOWARN);
> +	if (!btf->data) {
> +		err = -ENOMEM;
> +		goto errout;
> +	}
> +	btf->data_size = data_size;
> +
> +	err = btf_parse_hdr(env);
> +	if (err)
> +		goto errout;
> +
> +	btf->nohdr_data = btf->data + btf->hdr.hdr_len;
> +
> +	err = btf_parse_str_sec(env);
> +	if (err)
> +		goto errout;
> +
> +	err = btf_check_all_metas(env);
> +	if (err)
> +		goto errout;
> +
> +	err = btf_check_modifier_chain_length(env, btf, btf_nr_types(base_btf));
> +	if (err)
> +		goto errout;
> +
> +	return btf;
> +
> +errout:
> +	btf_free(btf);
> +	return ERR_PTR(err);
> +}

[ ... ]

> @@ -6759,67 +6884,75 @@ static struct btf *btf_parse_module(const char *module_name, const void *data,
>  				    unsigned int data_size, void *base_data,
> -				    unsigned int base_data_size)
> +				    unsigned int base_data_size, const void *inline_data,
> +				    unsigned int inline_data_size, bool vmlinux_inline,
> +				    void **relocated_inline_data)
>  {
> -	struct btf *btf = NULL, *vmlinux_btf, *base_btf = NULL;
> +	struct btf *btf = NULL, *inline_btf = NULL, *vmlinux_btf, *base_btf = NULL;
> +	struct btf *inline_base_btf;
>  	struct btf_verifier_env *env = NULL;
>  	struct bpf_verifier_log *log;
> +	u32 old_module_type_cnt;
> +	u32 *module_str_map = NULL;
>  	int err = 0;

[ ... ]

> -	btf = kzalloc_obj(*btf, GFP_KERNEL | __GFP_NOWARN);
> -	if (!btf) {
> -		err = -ENOMEM;
> +	btf = btf_parse_split(env, module_name, data, data_size, base_btf);
> +	if (IS_ERR(btf)) {
> +		err = PTR_ERR(btf);
> +		btf = NULL;
>  		goto errout;
>  	}

[ ... ]

> +	if (inline_data_size) {
> +		/*
> +		 * Ordinary module inline BTF is split relative to the module BTF.
> +		 * The btf_vmlinux_inline delivery module instead carries BTF split
> +		 * directly relative to vmlinux.
> +		 */
> +		inline_base_btf = vmlinux_inline ? vmlinux_btf : btf;
> +		inline_btf = btf_parse_split(env, module_name, inline_data,
> +					     inline_data_size, inline_base_btf);
> +		if (IS_ERR(inline_btf)) {
> +			pr_warn("failed to validate module [%s] inline BTF: %ld\n",
> +				module_name, PTR_ERR(inline_btf));
> +			inline_btf = NULL;
> +		}
> +	}

[ ... ]

>  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);
>  	return ERR_PTR(err);
>  }

Can btf_free() handle NULL pointers safely? Both inline_btf and btf are
initialized to NULL and can remain NULL on several error paths:

btf_parse_module() {
    struct btf *btf = NULL, *inline_btf = NULL, ...

If btf_parse_split() fails for the module BTF at the call site above,
the code sets btf = NULL and jumps to errout, which then calls:

errout:
    btf_free(inline_btf);  /* NULL */
    ...
    btf_free(btf);         /* NULL */

Looking at btf_free(), its first statement is
btf_free_struct_meta_tab(btf), which dereferences btf->struct_meta_tab
without a NULL check. Similarly, inline_btf is only assigned inside the
if (inline_data_size) block, so it remains NULL on modules without inline
BTF or when inline parsing fails.

The pre-patch code guarded the btf free with:

    if (btf) {
        kvfree(btf->data);
        kvfree(btf->types);
        kfree(btf);
    }

while this patch replaces it with bare btf_free() calls. btf_parse() in
this same file still uses the guarded pattern: if (btf) btf_free(btf).

Three paths that can trigger this:

1. btf_parse_split() fails for the module BTF due to -ENOMEM from
kvmemdup, bad BTF magic/version in btf_parse_hdr(), malformed type
metadata in btf_check_all_metas(), or -ELOOP from
btf_check_modifier_chain_length(). Both btf and inline_btf are NULL at
errout.

2. btf_parse_base(".BTF.base", ...) fails for an out-of-tree module.
Both btf and inline_btf are still NULL.

3. btf_relocate() fails on a module with no .BTF.inline section, or whose
inline parse already set inline_btf = NULL. btf_free(NULL) at errout.

CONFIG_MODULE_ALLOW_BTF_MISMATCH exists so BTF validation failure
degrades gracefully instead of failing module load, but with this patch
the errout path would crash first.


---
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/33537080133