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

[Ihor Solodrai <ihor.solodrai@linux.dev>]

On 7/10/26 10:23 AM, Kumar Kartikeya Dwivedi wrote:
> On Fri Jul 10, 2026 at 7:08 PM CEST, Ihor Solodrai wrote:
>> On 7/9/26 6:21 PM, sashiko-bot@kernel.org wrote:
>>> 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;
>>>
> 
> Also, why not do it for BPF_TRACE_ITER case as well? I just feel keeping it
> consistent may outweigh figuring out whether it's necessary in that case or not,
> etc.


I mentioned this in the patch description:

  The BPF_TRACE_ITER case distills a registered iterator's
  prototype, and bpf_struct_ops_desc_init() distills the
  function-pointer members of a struct_ops type. Neither is 
  a kfunc, and so can't have implicit arguments.


> 
> Please supply a cover letter in the next version too since you're sending
> multiple patches.

I used the first patch's message as a cover letter, since it's
just a fix + selftest pair.

The series could be split further into refactoring changes, but I thought
it'll be easier to backport if the fix is in a single self-contained patch.

I think we want the fix to get into 7.2, and backport it to 7.1.

If it's ok to carry separate refactoring patches there, then I'll split.
Kartikeya, wdyt?


> 
> Thanks