[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

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

[Jérémy Jean]

Hello Alexei,

On 2026-08-26 03:22, Alexei Starovoitov wrote:
> 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.

> I'm pretty sure we have selftests for negative
> error codes from res_spin_lock.

As far as I could checked, this is not entirely covered: when the
returned 32-bit signed int is a negative integer in %eax
(0xffffffdd), it is later mapped to 64-bit register r0, which clears
the 32 MSB as its value is taken from %rax, and this makes the
negative error code a large 32-bit value stored in r0
(0x00000000ffffffdd). When writing raw BPF, one could check the
return code using r0 instead of w0, and this would introduce a
mismatch with the verifier that sees a real negative value.

In my understanding, the current selftests do not exactly verify this?

> Pls craft a selftest if the issue is real.

Below is an attempt for a selftest. I got help from an LLM to write
the BPF instructions, and then try my best to shrink it but it is a
bit long; hope that's okay. The point is to craft the check on r0
manually after the call to bpf_res_spin_lock(), and forces the branch
taken to be different between the verifier and the jitted code.

The need for raw BPF instructions is required as the compilation from
C correctly uses w0 for the check. I added comments to emphasize this.

SEC("?tc")
__arch_x86_64
__load_if_JITed()
__success
__retval(0)
__naked int res_spin_lock_return_sign_extension(void)
{
   asm volatile ("                                                       
\
   *(u32 *)(r10 - 4) = 0;                                                
\
   r2 = r10;                                                             
\
   r2 += -4;                                                             
\
   r9 = r2;                                                              
\
   r1 = %[arrmap] ll;                                                    
\
   call %[bpf_map_lookup_elem];                                          
\
   if r0 == 0 goto out_%=;                                               
\
   r6 = r0;                                                              
\
   /* Adversarial setup: give one map value two verifier pointer IDs. */ 
\
   r2 = r9;                                                              
\
   r1 = %[arrmap] ll;                                                    
\
   call %[bpf_map_lookup_elem];                                          
\
   if r0 == 0 goto out_%=;                                               
\
   r7 = r0;                                                              
\
   r8 = 0;                                                               
\
   r1 = r6;                                                              
\
   call %[bpf_res_spin_lock];                                            
\
   if w0 != 0 goto out_%=;                                               
\
   r1 = r7;                                                              
\
   call %[bpf_res_spin_lock];                                            
\
   /* Adversarial check: treat the negative int return as 64-bit R0. */  
\
   if r0 s< 0 goto unlock_%=;                                            
\
   r8 = 1;                                                               
\
   if r0 != 0 goto unlock_%=;                                            
\
   r1 = r7;                                                              
\
   call %[bpf_res_spin_unlock];                                          
\
unlock_%=:                                                              
\
   r1 = r6;                                                              
\
   call %[bpf_res_spin_unlock];                                          
\
   r0 = r8;                                                              
\
   exit;                                                                 
\
out_%=:                                                                 
\
   exit;                                                                 
\
"  :
  : __imm(bpf_map_lookup_elem),
    __imm(bpf_res_spin_lock),
    __imm(bpf_res_spin_unlock),
    __imm_addr(arrmap)
  : __clobber_all);
}

When I run the selftest (I used 9328b3b03bdc), I get:

#1/1     res_spin_lock_failure/res_spin_lock_arg:OK
#1/2     res_spin_lock_failure/res_spin_lock_AA:OK
#1/3     res_spin_lock_failure/res_spin_lock_cond_AA:OK
#1/4     res_spin_lock_failure/res_spin_lock_mismatch_1:OK
#1/5     res_spin_lock_failure/res_spin_lock_mismatch_2:OK
#1/6     res_spin_lock_failure/res_spin_lock_irq_mismatch_1:OK
#1/7     res_spin_lock_failure/res_spin_lock_irq_mismatch_2:OK
#1/8     res_spin_lock_failure/res_spin_lock_ooo:OK
#1/9     res_spin_lock_failure/res_spin_lock_ooo_irq:OK
(..snip..)
#1/10    res_spin_lock_failure/pin_lock_return_sign_extension:FAIL
#1/11    res_spin_lock_failure/res_spin_lock_ooo_unlock:OK
#1/12    res_spin_lock_failure/res_spin_lock_bad_off:OK
#1/13    res_spin_lock_failure/res_spin_lock_var_off:OK
#1/14    res_spin_lock_failure/res_spin_lock_no_lock_map:OK
#1/15    res_spin_lock_failure/res_spin_lock_no_lock_kptr:OK

If you confirm this is not yet covered in the selftests, I can send a
two-commit patch (one with the previous patch, one with the selftest).

Regards,
Jérémy