All of lore.kernel.org
 help / color / mirror / Atom feed
From: Vineet Gupta <vineet.gupta@linux.dev>
To: Eduard Zingerman <eddyz87@gmail.com>, bpf@vger.kernel.org
Cc: ast@kernel.org, daniel@iogearbox.net, andrii@kernel.org,
	x86@kernel.org, stable@vger.kernel.org
Subject: Re: [PATCH] bpf, x86: Fix per-CPU address resolution into an extended register
Date: Fri, 14 Aug 2026 12:45:24 -0700	[thread overview]
Message-ID: <a3d7bef2-4168-4804-b92d-cdc1d0184584@linux.dev> (raw)
In-Reply-To: <5acb321999d7cc7ebd945c519495a8762501e3cc.camel@gmail.com>

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: stable@vger.kernel.org
>> Signed-off-by: Vineet Gupta <vineet.gupta@linux.dev>
>> ---
>>   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 <eddyz87@gmail.com>

Thanks for the quick review !

-Vineet

      reply	other threads:[~2026-08-14 19:45 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-14 16:55 [PATCH] bpf, x86: Fix per-CPU address resolution into an extended register Vineet Gupta
2026-08-14 19:13 ` Eduard Zingerman
2026-08-14 19:45   ` Vineet Gupta [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=a3d7bef2-4168-4804-b92d-cdc1d0184584@linux.dev \
    --to=vineet.gupta@linux.dev \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=eddyz87@gmail.com \
    --cc=stable@vger.kernel.org \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.