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

On 8/14/26 2:20 PM, [email protected] wrote:
>> bpf, x86: Fix per-CPU address resolution into an extended register
>>
>> 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]>
> Should this include Eduard Zingerman's Reviewed-by tag? In the v2 review
> thread, Eduard provided "Reviewed-by: Eduard Zingerman <[email protected]>"
> and specifically noted "Please don't drop acks."
>
> https://lore.kernel.org/bpf/[email protected]/

Yes I missed it. Will carry it in v3.

>> diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
>> index de7515ea1bea..276d076d2993 100644
>> --- a/arch/x86/net/bpf_jit_comp.c
>> +++ b/arch/x86/net/bpf_jit_comp.c
>> @@ -1887,7 +1887,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));
>>   				EMIT3(0x03, add_2reg(0x04, 0, dst_reg), 0x25);
>>   				EMIT((u32)(unsigned long)&this_cpu_off, 4);
>>   #endif
> In the v1 review thread, Eduard requested an inline assembly test with
> __xlated() matcher to verify the JIT generates correct code for per-CPU
> address resolution. A test was apparently added in v2 as patch 2/2
> (tools/testing/selftests/bpf/progs/verifier_percpu_addr.c), but that test is
> not present in this commit.
>
> Given that this bug affects all four extended registers (R5, R7, R8, R9)
> and can cause kernel panics or frame pointer corruption, should the test be
> included here to verify the fix works correctly?
>
> https://lore.kernel.org/bpf/[email protected]/

Per BPF convention tests are generally separate patch and it is in 2/2 
of this series.

Thx,
-Vineet

>
>
> ---
> 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/31839526403