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

On 8/14/26 12:13 PM, Eduard Zingerman wrote:
> 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?

xlated or jited ?

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

Thanks for the quick review !

-Vineet