Re: [PATCH bpf v1 1/2] bpf: Fix tracing of kfuncs with implicit args

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Accessing `btf->kfunc_set_tab` without holding a module reference causes a use-after-free vulnerability.
--

--- Patch [1]: [PATCH bpf v1 1/2] bpf: Fix tracing of kfuncs with implicit args ---
commit 13df91bf669b3a55c459c1663fbf726f9f52bc5c
Author: Ihor Solodrai <ihor.solodrai@linux.dev>

bpf: Fix tracing of kfuncs with implicit args

A kfunc marked with KF_IMPLICIT_ARGS flag takes implicit arguments
(such as bpf_prog_aux) that the verifier injects at load time.
resolve_btfids strips those from the kfunc's BTF-visible prototype and
keeps the real kernel ABI in a counterpart _impl prototype.

Link: https://github.com/sched-ext/scx/issues/3687#issuecomment-4906694106

> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index 64572f85edc83..20aeca6b4f95c 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
[ ... ]
> @@ -9114,6 +9114,26 @@ u32 *btf_kfunc_flags(const struct btf *btf, u32 kfunc_btf_id, const struct bpf_p
>  	return btf_kfunc_id_set_contains(btf, hook, kfunc_btf_id);
>  }
>  
> +/*
> + * Return the union of a kfunc's flags across all hooks.
> + * Unlike btf_kfunc_flags(), not restricted to a calling
> + * program's hook. Used when attaching to a kfunc for tracing.
> + */
> +u32 btf_kfunc_accumulated_flags(const struct btf *btf, u32 kfunc_btf_id)
> +{
> +	enum btf_kfunc_hook hook;
> +	u32 *hook_flags;
> +	u32 flags = 0;
> +
> +	for (hook = 0; hook < BTF_KFUNC_HOOK_MAX; hook++) {
> +		hook_flags = btf_kfunc_id_set_contains(btf, hook, kfunc_btf_id);
> +		if (hook_flags)
> +			flags |= *hook_flags;
> +	}
> +
> +	return flags;
> +}
> +
>  u32 *btf_kfunc_is_modify_return(const struct btf *btf, u32 kfunc_btf_id,
>  				const struct bpf_prog *prog)
>  {
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 6515d4d3c0032..fde11a2f6869a 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
[ ... ]
> @@ -19121,8 +19146,8 @@ int bpf_check_attach_target(struct bpf_verifier_log *log,
>  		if (prog_extension &&
>  		    btf_check_type_match(log, prog, btf, t))
>  			return -EINVAL;
> -		t = btf_type_by_id(btf, t->type);
> -		if (!btf_type_is_func_proto(t))
> +		t = btf_attach_func_proto(log, btf, btf_id);
> +		if (!t || !btf_type_is_func_proto(t))
>  			return -EINVAL;

[Severity: High]
Does this introduce a use-after-free race condition during module load?

The new btf_attach_func_proto() call executes before the module reference is
safely acquired via btf_try_get_module() later in bpf_check_attach_target():

bpf_check_attach_target()
  btf_attach_func_proto()
    btf_kfunc_accumulated_flags()
      btf_kfunc_id_set_contains()
        // Accesses btf->kfunc_set_tab->sets[hook]

If a module is just loading (in MODULE_STATE_COMING) and its BTF is already
visible, a concurrent BPF_PROG_LOAD could trigger this path. Meanwhile, the
module's init function might call register_btf_kfunc_id_set(), which uses
krealloc() on the same array.

Could the sets array be freed by krealloc() while the verifier is performing a
binary search on it here?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260710005902.2234832-1-ihor.solodrai@linux.dev?part=1