[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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The sign-extension fix for kfunc returns is implemented for x86-64 but omitted for other architectures (ARM64, PowerPC64, x86-32) where identical verifier/runtime divergence exists.
--

--- Patch [1]: [PATCH] bpf, x86: Sign-extend narrow signed kfunc returns ---
commit 9b3fa67779a40b052a3a12d689e1b9cac8ac02a9
Author: Jérémy Jean <Jeremy.Jean@oss.cyber.gouv.fr>

bpf, x86: Sign-extend narrow signed kfunc returns

This patch fixes an issue on x86-64 where narrow signed kfunc return values
are not properly sign-extended to 64 bits. This divergence between verifier
assumptions and runtime execution can lead to state corruption and unbalanced
locks. The JIT is updated to use the kfunc's BTF model to sign-extend these
returns into R0 after the native call.

> diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
> index 2853e87797a78..ec8a7694adb62 100644
> --- a/arch/x86/net/bpf_jit_comp.c
> +++ b/arch/x86/net/bpf_jit_comp.c

[ ... ]

> @@ -2665,6 +2682,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;
> +			}

[Severity: High]
This isn't a bug introduced by this patch, but it appears this same issue
is present in other architectures where the native calling convention leaves
the upper 32 bits undefined for 32-bit returns.

For example, in the ARM64 JIT:

arch/arm64/net/bpf_jit_comp.c:build_insn() {
    ...
    if (insn->src_reg == BPF_PSEUDO_KFUNC_CALL) {
        ret = emit_kfunc_arena_args(ctx, insn);
        if (ret < 0)
            return ret;
    }
    emit_call(func_addr, ctx);
    ...
}

The ARM64 JIT lacks post-call return value sign-extension, leaving the upper
32 bits of X0 undefined per the AAPCS64.

Similarly, in PowerPC64:

arch/powerpc/net/bpf_jit_comp64.c:bpf_jit_build_body() {
    ...
    ret = bpf_jit_emit_func_call_rel(image, fimage, ctx, func_addr);
    if (ret)
        return ret;
    /* move return value from r3 to BPF_REG_0 */
    EMIT(PPC_RAW_MR(bpf_to_ppc(BPF_REG_0), _R3));
    ...
}

And on x86-32, the upper 32 bits of BPF_REG_0 are left uninitialized
on the stack for 32-bit and narrower returns, leaving stale data from
before the call:

arch/x86/net/bpf_jit_comp32.c:emit_kfunc_call() {
    ...
    if (fm->ret_size > sizeof(u32))
        /* mov dword ptr [ebp+off],edx */
        EMIT3(0x89, add_2reg(0x40, IA32_EBP, IA32_EDX),
              STACK_VAR(bpf2ia32[BPF_REG_0][1]));
    ...
}

Since a BPF program calling a kfunc that returns a narrow signed type
(like bpf_res_spin_lock) would also experience the verifier/runtime divergence
on these architectures, does this sign-extension fix need to be applied
there as well to prevent state corruption?

>  			if (priv_frame_ptr)
>  				pop_r9(&prog);
>  			break;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260823203944.2084994-2-Jeremy.Jean@oss.cyber.gouv.fr?part=1

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