Re: [RESEND PATCH bpf-next v6 5/7] bpf: Directly store kfunc desc index in instruction off field

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] bpf: JIT compilation fails for kfuncs emitted in prologues
--

commit ba562be891ad9f52f9742001cfab2a791d6aefb8
Author: Emil Tsalapatis <emil@etsalapatis.com>

bpf: Directly store kfunc desc index in instruction off field
    
This patch modifies the verifier to store the kfunc descriptor index directly
in the instruction's off field. It removes the need to sort the descriptor
table twice, simplifying subsequent function specialization patches.

> diff --git a/kernel/bpf/fixups.c b/kernel/bpf/fixups.c
> index 37cf130ebb57b..cb7219ff68bd1 100644
> --- a/kernel/bpf/fixups.c
> +++ b/kernel/bpf/fixups.c

[ ... ]

>  const struct btf_func_model *
>  bpf_jit_find_kfunc_model(const struct bpf_prog *prog,
>  			 const struct bpf_insn *insn)
>  {
[ ... ]
>  	tab = prog->aux->kfunc_tab;
> -	res = bsearch(&desc, tab->descs, tab->nr_descs,
> -		      sizeof(tab->descs[0]), kfunc_desc_cmp_by_imm_off);
> -
> -	return res ? &res->func_model : NULL;
> -}
[ ... ]
> +	if (insn->off < 0 || insn->off >= tab->nr_descs)
> +		return NULL;
>  
> +	res = &tab->descs[insn->off];
>  	if (bpf_jit_supports_far_kfunc_call()) {
> +		if (res->func_id != insn->imm)
> +			return NULL;

[Severity: Medium]
Will this lead to a JIT failure for kfuncs emitted in prologues or
epilogues?

When bpf_convert_ctx_accesses() calls gen_prologue(), it can emit a kfunc
call where insn->off is 0. This is then processed by add_kfunc_in_insns(),
which calls bpf_add_kfunc_call() to register the kfunc:

kernel/bpf/fixups.c:add_kfunc_in_insns() {
	...
	for (i = 0; i < cnt; i++, insn++) {
		if (bpf_pseudo_kfunc_call(insn)) {
			ret = bpf_add_kfunc_call(env, insn->imm, insn->off);
			if (ret < 0)
				return ret;
		}
	}
	...
}

However, add_kfunc_in_insns() does not update insn->off to the descriptor
index returned by bpf_add_kfunc_call().

Later, when the JIT compiler calls bpf_jit_find_kfunc_model() and uses
insn->off (which is still 0) to fetch the descriptor, could it fetch the
wrong descriptor and abort the compilation due to a func_id mismatch?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261002105218.6171-1-emil@etsalapatis.com?part=5