Re: [PATCH] bpf, x86: Sign-extend narrow signed kfunc returns

[bot+bpf-ci]

> diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
> index 48429fae0641..93b9e36df1d2 100644
> --- a/arch/x86/net/bpf_jit_comp.c
> +++ b/arch/x86/net/bpf_jit_comp.c
> @@ -1721,6 +1721,23 @@ static int emit_kfunc_arena_args(struct bpf_prog *bpf_prog,
>  	return prog - start;
>  }
>
> +static int emit_kfunc_return(const struct bpf_prog *bpf_prog,
> +			     const struct bpf_insn *insn, u8 **pprog)
> +{
> +	const struct btf_func_model *fm;
> +
> +	fm = bpf_jit_find_kfunc_model(bpf_prog, insn);
> +	if (!fm)
> +		return -EINVAL;
> +	if (!(fm->ret_flags & BTF_FMODEL_SIGNED_ARG) || fm->ret_size == 8)
> +		return 0;
> +	if (fm->ret_size != 1 && fm->ret_size != 2 && fm->ret_size != 4)
                    ^^^^

Does this reject supported kfunc return sizes?  The early-out skips only
fm->ret_size == 8, so a signed return of size 16 falls through to this
check and triggers -EINVAL.

Size 16 is a valid kfunc return size:

kernel/bpf/btf.c:__get_type_size() returns t->size for any
btf_type_is_int(), and btf_distill_func_proto() only rejects ret > 16, so
fm->ret_size legitimately reaches 16.

kernel/bpf/btf.c:__get_type_fmodel_flags() sets BTF_FMODEL_SIGNED_ARG for
every BTF_KIND_INT carrying BTF_INT_SIGNED. __int128 is encoded exactly
that way (BTF_KIND_INT, size 16, BTF_INT_SIGNED), so both the SIGNED flag
and ret_size 16 are set together.

do_jit() already handles the wide case just below this call site:

	if (fm && fm->ret_size > 8)
		emit_mov_reg(&prog, true, BPF_REG_2, BPF_REG_3);

Concrete in-tree instance at
tools/testing/selftests/bpf/test_kmods/bpf_testmod.c:

#if defined(__x86_64__) || defined(__aarch64__)
__bpf_kfunc __int128 bpf_kfunc_call_test_i128(u64 a, u64 b)

exercised by tools/testing/selftests/bpf/progs/verifier_aggregate_ret.c
(__arch_x86_64 __arch_arm64 __load_if_JITed() __success __retval(0)) and
tools/testing/selftests/bpf/progs/aggregate_ret_kfunc.c.

emit_kfunc_return() returning -EINVAL causes do_jit() to return -EINVAL.
In bpf_int_jit_compile(), this takes the proglen <= 0 branch, which frees
the image and clears prog->jited. On the common CONFIG_BPF_JIT_ALWAYS_ON=y
configuration, program load fails outright instead of falling back to the
interpreter. Any BPF program calling a kfunc with a signed 16-byte return
stops loading on x86-64.

Should the first check use fm->ret_size >= 8 instead of fm->ret_size == 8
to keep the whitelist meaningful while preserving the existing wide-return
behaviour?

> +		return -EINVAL;
> +
> +	emit_movsx_reg(pprog, fm->ret_size * 8, true, BPF_REG_0, BPF_REG_0);
> +	return 0;
> +}
> +
>  static int do_jit(struct bpf_verifier_env *env, struct bpf_prog *bpf_prog, int *addrs, u8 *image,
>  		  u8 *rw_image, int oldproglen, struct jit_context *ctx, bool jmp_padding)
>  {
> @@ -2664,6 +2681,11 @@ st:			insn_off = insn->off;
>  			ip += x86_call_depth_emit_accounting(&prog, func, ip);
>  			if (emit_call(&prog, func, ip))
>  				return -EINVAL;
> +			if (src_reg == BPF_PSEUDO_KFUNC_CALL) {
> +				err = emit_kfunc_return(bpf_prog, insn, &prog);
> +				if (err)
> +					return err;
> +			}
>  			if (priv_frame_ptr)
>  				pop_r9(&prog);
>  			/*


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