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

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

[Jérémy Jean]

bpf_res_spin_lock() returns a 32-bit int. On failure, the verifier
models R0 as a signed 64-bit value in [-MAX_ERRNO, -1].

On x86-64, returning -EDEADLK writes 0xffffffdd to EAX and clears the
upper half of RAX. Since the JIT leaves the native return value as-is,
BPF sees 0x00000000ffffffdd instead of the sign-extended
0xffffffffffffffdd. A 64-bit signed comparison therefore treats the
value as positive, while the verifier treats it as -35.

As a result, a signed comparison against zero can take one path during
verification and another at run time. With rqspinlock aliases, this can
lead to unmatched bpf_res_spin_unlock() calls, corrupting the per-CPU
rqspinlock state and unbalancing the preemption count.

Use the kfunc's BTF model to sign-extend signed 8-, 16-, and 32-bit
returns into R0 after the native call.

Fixes: 0de2046137f9 ("bpf: Implement verifier support for rqspinlock")
Assisted-by: Codex:gpt-5
Signed-off-by: Jérémy Jean <Jeremy.Jean@oss.cyber.gouv.fr>
---
 arch/x86/net/bpf_jit_comp.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
index 1a9fb530adc3..713ac70613ff 100644
--- a/arch/x86/net/bpf_jit_comp.c
+++ b/arch/x86/net/bpf_jit_comp.c
@@ -1726,6 +1726,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)
+		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);
 			break;
-- 
2.47.3

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

[bot+bpf-ci]

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

> 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

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

[Alexei Starovoitov]

On Sun Aug 23, 2026 at 1:39 PM PDT, Jérémy Jean wrote:
> bpf_res_spin_lock() returns a 32-bit int. On failure, the verifier
> models R0 as a signed 64-bit value in [-MAX_ERRNO, -1].
>
> On x86-64, returning -EDEADLK writes 0xffffffdd to EAX and clears the
> upper half of RAX. Since the JIT leaves the native return value as-is,
> BPF sees 0x00000000ffffffdd instead of the sign-extended
> 0xffffffffffffffdd. A 64-bit signed comparison therefore treats the
> value as positive, while the verifier treats it as -35.
>
> As a result, a signed comparison against zero can take one path during
> verification and another at run time. With rqspinlock aliases, this can
> lead to unmatched bpf_res_spin_unlock() calls, corrupting the per-CPU
> rqspinlock state and unbalancing the preemption count.

Is that a theory? I'm pretty sure we have selftests for negative
error codes from res_spin_lock.
Pls craft a selftest if the issue is real.

pw-bot: cr