Re: [PATCH] bpf, x86: Fix per-CPU address resolution into an extended register

On Fri, 2026-08-14 at 09:55 -0700, Vineet Gupta wrote:
> The destination of the per-CPU address MOV is encoded in ModRM.reg,
> which is extended by REX.R, but the REX prefix is built with
> add_1mod(), which sets REX.B. REX.B extends ModRM.rm and SIB.base, and
> this instruction addresses memory as disp32 with no base, so the bit
> has no effect at all and the high register bit is simply lost.
> 
> Every is_ereg() destination therefore resolves to the wrong register,
> picking whichever one shares the low three bits:
> 
>   R5 -> RAX    R7 -> RBP    R8 -> RSI    R9 -> RDI
> 
> With BPF_REG_5, whose reg2hex is 0, the emitted
> 
>   65 49 03 04 25 <off>	add %gs:<off>,%rax
> 
> adds the per-CPU offset to RAX rather than R8. The destination keeps
> the unadjusted address and RAX is clobbered, so the program goes on to
> dereference a pointer that was never made per-CPU:
> 
>   BUG: unable to handle page fault for address: 0000607e386a8894
>   RIP: bpf_prog_707837aafd2aa9ae_update_percpu_data+0x93/0xc9
>   Call Trace:
>    __bpf_prog_test_run_raw_tp+0x2dc/0x7d0
>    __flush_smp_call_function_queue+0x1e9/0xc80
>   Kernel panic - not syncing: Fatal exception in interrupt
> 
> R5 is the mildest of the four, aliasing a scratch register and faulting
> at the store. R7 aliases RBP and would corrupt the frame pointer, R8
> and R9 alias the argument registers.
> 
> Use add_2mod() so the register goes through REX.R, matching how
> add_2reg() places it in ModRM.reg and how emit_priv_frame_ptr()
> hardcodes 0x4c for the same instruction with R9. Encodings for the
> non-extended registers are unchanged.
> 
> Problem showed up when trying to resurrect BPF_GCC CI (selftests built
> with BPF_GCC).
> 
> This has gone unnoticed because clang reloads the address into R1
> before each per-CPU access, so the destination is never an extended
> register. GCC keeps several per-CPU addresses live at once, and
> test_progs-bpf_gcc panics the kernel in global_percpu_data/init, where
> the address of a .percpu variable ends up in R5.
> 
> Fixes: 7bdbf7446305 ("bpf: add special internal-only MOV instruction to resolve per-CPU addrs")
> Cc: [email protected]
> Signed-off-by: Vineet Gupta <[email protected]>
> ---
>  arch/x86/net/bpf_jit_comp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
> index d920772af7d5..1a9fb530adc3 100644
> --- a/arch/x86/net/bpf_jit_comp.c
> +++ b/arch/x86/net/bpf_jit_comp.c
> @@ -1935,7 +1935,7 @@ static int do_jit(struct bpf_verifier_env *env, struct bpf_prog *bpf_prog, int *
>  				EMIT_mov(dst_reg, src_reg);
>  #ifdef CONFIG_SMP
>  				/* add <dst>, gs:[<off>] */
> -				EMIT2(0x65, add_1mod(0x48, dst_reg));
> +				EMIT2(0x65, add_2mod(0x48, 0, dst_reg));

Ok, so the argument is that because there is a SIB byte in this
instruction, and the instruction itself is `REX.W + 03 /r | ADD r64, r/m64`,
operand #1 ModRM:reg, operand #2 ModRM:r/m, the encoding should adjust
REX.R to extend operand #1, not REX.B as it does now.

I think this is correct. Could you please respin adding an inline
assembly test with __xlated() matcher, to verify that jit operates
correctly?

Reviewed-by: Eduard Zingerman <[email protected]>

>  				EMIT3(0x03, add_2reg(0x04, 0, dst_reg), 0x25);
>  				EMIT((u32)(unsigned long)&this_cpu_off, 4);
>  #endif