* [PATCH bpf v2 0/2] bpf, x86: fix per-CPU address resolution into an extended register
@ 2026-08-14 20:32 Vineet Gupta
2026-08-14 20:32 ` [PATCH bpf v2 1/2] bpf, x86: Fix " Vineet Gupta
2026-08-14 20:32 ` [PATCH bpf v2 2/2] selftests/bpf: Check per-CPU address resolution per register Vineet Gupta
0 siblings, 2 replies; 9+ messages in thread
From: Vineet Gupta @ 2026-08-14 20:32 UTC (permalink / raw)
To: bpf; +Cc: ast, daniel, andrii, x86, stable, Vineet Gupta
The JIT resolves a per-CPU address with
add <dst>, gs:[this_cpu_off]
but builds the REX prefix with add_1mod(), which sets REX.B. The
destination is encoded in ModRM.reg, which REX.R extends, and the memory
operand is disp32 with no base, so REX.B does nothing and the high
register bit is dropped. Every extended destination therefore resolves
into whichever register shares the low three bits:
R5 -> RAX R7 -> RBP R8 -> RSI R9 -> RDI
The address is left unadjusted and an unrelated register is clobbered.
Patch 1 switches to add_2mod() so the bit goes through REX.R.
Clang reloads the address into R1 before each per-CPU access, so the
destination is never an extended register and the bug has been dormant
since v6.10. GCC keeps several per-CPU addresses live at once, which is
how it turned up: test_progs-bpf_gcc panics the kernel in
global_percpu_data/init, with the address of a .percpu variable in R5.
Patch 2 adds a test per register. A functional test only catches this if
the address happens to land in an extended register, so the test matches
the JITed add instead, one __naked program per register.
Changes in v2:
- Add the selftest, patch 2/2 (Eduard Zingerman). It uses __jited()
rather than __xlated(): the xlated stream is identical for every
register, and the wrong prefix is only visible in the native encoding.
- No functional change to patch 1.
v1: https://lore.kernel.org/bpf/20260814165557.3405518-1-vineet.gupta@linux.dev/T/#u
Vineet Gupta (2):
bpf, x86: Fix per-CPU address resolution into an extended register
selftests/bpf: Check per-CPU address resolution per register
arch/x86/net/bpf_jit_comp.c | 2 +-
.../selftests/bpf/prog_tests/verifier.c | 2 +
.../bpf/progs/verifier_percpu_addr.c | 114 ++++++++++++++++++
3 files changed, 117 insertions(+), 1 deletion(-)
create mode 100644 tools/testing/selftests/bpf/progs/verifier_percpu_addr.c
--
2.53.0-Meta
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH bpf v2 1/2] bpf, x86: Fix per-CPU address resolution into an extended register
2026-08-14 20:32 [PATCH bpf v2 0/2] bpf, x86: fix per-CPU address resolution into an extended register Vineet Gupta
@ 2026-08-14 20:32 ` Vineet Gupta
2026-08-14 20:44 ` Eduard Zingerman
2026-08-14 21:20 ` bot+bpf-ci
2026-08-14 20:32 ` [PATCH bpf v2 2/2] selftests/bpf: Check per-CPU address resolution per register Vineet Gupta
1 sibling, 2 replies; 9+ messages in thread
From: Vineet Gupta @ 2026-08-14 20:32 UTC (permalink / raw)
To: bpf; +Cc: ast, daniel, andrii, x86, stable, Vineet Gupta
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));
EMIT3(0x03, add_2reg(0x04, 0, dst_reg), 0x25);
EMIT((u32)(unsigned long)&this_cpu_off, 4);
#endif
--
2.53.0-Meta
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH bpf v2 2/2] selftests/bpf: Check per-CPU address resolution per register
2026-08-14 20:32 [PATCH bpf v2 0/2] bpf, x86: fix per-CPU address resolution into an extended register Vineet Gupta
2026-08-14 20:32 ` [PATCH bpf v2 1/2] bpf, x86: Fix " Vineet Gupta
@ 2026-08-14 20:32 ` Vineet Gupta
2026-08-14 20:49 ` Eduard Zingerman
2026-08-14 21:20 ` bot+bpf-ci
1 sibling, 2 replies; 9+ messages in thread
From: Vineet Gupta @ 2026-08-14 20:32 UTC (permalink / raw)
To: bpf; +Cc: ast, daniel, andrii, x86, stable, Vineet Gupta
An ld_imm64 of a per-CPU map value is followed by a mov_percpu_addr that
reuses the same register, so which register the address lands in
decides how the JIT encodes the add. Getting the REX prefix wrong there
is invisible to a functional test on x86 unless the address happens to
land in an extended register, which is why this went unnoticed.
Add one __naked program per extended register, R5, R7, R8 and R9, each
loading a .percpu variable into that register, and match the JITed add
against the register it must resolve into. R1 is covered too, so that a
fix which sets REX.R unconditionally does not pass either.
Signed-off-by: Vineet Gupta <vineet.gupta@linux.dev>
---
.../selftests/bpf/prog_tests/verifier.c | 2 +
.../bpf/progs/verifier_percpu_addr.c | 114 ++++++++++++++++++
2 files changed, 116 insertions(+)
create mode 100644 tools/testing/selftests/bpf/progs/verifier_percpu_addr.c
diff --git a/tools/testing/selftests/bpf/prog_tests/verifier.c b/tools/testing/selftests/bpf/prog_tests/verifier.c
index 8113fea7ba86..64ac49ad67e6 100644
--- a/tools/testing/selftests/bpf/prog_tests/verifier.c
+++ b/tools/testing/selftests/bpf/prog_tests/verifier.c
@@ -79,6 +79,7 @@
#include "verifier_netfilter_retcode.skel.h"
#include "verifier_bpf_fastcall.skel.h"
#include "verifier_or_jmp32_k.skel.h"
+#include "verifier_percpu_addr.skel.h"
#include "verifier_precision.skel.h"
#include "verifier_prevent_map_lookup.skel.h"
#include "verifier_private_stack.skel.h"
@@ -240,6 +241,7 @@ void test_verifier_netfilter_ctx(void) { RUN(verifier_netfilter_ctx); }
void test_verifier_netfilter_retcode(void) { RUN(verifier_netfilter_retcode); }
void test_verifier_bpf_fastcall(void) { RUN(verifier_bpf_fastcall); }
void test_verifier_or_jmp32_k(void) { RUN(verifier_or_jmp32_k); }
+void test_verifier_percpu_addr(void) { RUN(verifier_percpu_addr); }
void test_verifier_precision(void) { RUN(verifier_precision); }
void test_verifier_prevent_map_lookup(void) { RUN(verifier_prevent_map_lookup); }
void test_verifier_private_stack(void) { RUN(verifier_private_stack); }
diff --git a/tools/testing/selftests/bpf/progs/verifier_percpu_addr.c b/tools/testing/selftests/bpf/progs/verifier_percpu_addr.c
new file mode 100644
index 000000000000..962faea8ef90
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/verifier_percpu_addr.c
@@ -0,0 +1,114 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <vmlinux.h>
+#include <bpf/bpf_helpers.h>
+#include "bpf_misc.h"
+
+int percpu_data SEC(".percpu");
+
+#if defined(__TARGET_ARCH_x86)
+
+/*
+ * An ld_imm64 of a per-CPU map value is followed by a mov_percpu_addr that
+ * reuses the same register, so the register the address lands in decides how
+ * the JIT encodes the add. On x86 R5, R7, R8 and R9 are the extended
+ * registers, whose high bit needs REX.R because the destination sits in
+ * ModRM.reg. Check one program per extended register, since getting the
+ * prefix wrong resolves the address into whichever register shares the low
+ * three bits instead.
+ */
+
+SEC("raw_tp")
+__description("per-CPU address into r5")
+__success
+__arch_x86_64
+__jited(" addq %gs:{{.*}}, %r8")
+__naked void percpu_addr_into_r5(void)
+{
+ asm volatile (" \
+ r5 = %[percpu_data] ll; \
+ r0 = *(u32 *)(r5 + 0); \
+ exit; \
+" :
+ : __imm_addr(percpu_data)
+ : __clobber_all);
+}
+
+SEC("raw_tp")
+__description("per-CPU address into r7")
+__success
+__arch_x86_64
+__jited(" addq %gs:{{.*}}, %r13")
+__naked void percpu_addr_into_r7(void)
+{
+ asm volatile (" \
+ r7 = %[percpu_data] ll; \
+ r0 = *(u32 *)(r7 + 0); \
+ exit; \
+" :
+ : __imm_addr(percpu_data)
+ : __clobber_all);
+}
+
+SEC("raw_tp")
+__description("per-CPU address into r8")
+__success
+__arch_x86_64
+__jited(" addq %gs:{{.*}}, %r14")
+__naked void percpu_addr_into_r8(void)
+{
+ asm volatile (" \
+ r8 = %[percpu_data] ll; \
+ r0 = *(u32 *)(r8 + 0); \
+ exit; \
+" :
+ : __imm_addr(percpu_data)
+ : __clobber_all);
+}
+
+SEC("raw_tp")
+__description("per-CPU address into r9")
+__success
+__arch_x86_64
+__jited(" addq %gs:{{.*}}, %r15")
+__naked void percpu_addr_into_r9(void)
+{
+ asm volatile (" \
+ r9 = %[percpu_data] ll; \
+ r0 = *(u32 *)(r9 + 0); \
+ exit; \
+" :
+ : __imm_addr(percpu_data)
+ : __clobber_all);
+}
+
+/* A register that needs no REX.R, to catch a fix that overcorrects. */
+SEC("raw_tp")
+__description("per-CPU address into r1")
+__success
+__arch_x86_64
+__jited(" addq %gs:{{.*}}, %rdi")
+__naked void percpu_addr_into_r1(void)
+{
+ asm volatile (" \
+ r1 = %[percpu_data] ll; \
+ r0 = *(u32 *)(r1 + 0); \
+ exit; \
+" :
+ : __imm_addr(percpu_data)
+ : __clobber_all);
+}
+
+#else
+
+SEC("raw_tp")
+__description("percpu addr dummy")
+__success
+int dummy_test(void)
+{
+ return 0;
+}
+
+#endif
+
+char _license[] SEC("license") = "GPL";
--
2.53.0-Meta
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH bpf v2 1/2] bpf, x86: Fix per-CPU address resolution into an extended register
2026-08-14 20:32 ` [PATCH bpf v2 1/2] bpf, x86: Fix " Vineet Gupta
@ 2026-08-14 20:44 ` Eduard Zingerman
2026-08-14 21:20 ` bot+bpf-ci
1 sibling, 0 replies; 9+ messages in thread
From: Eduard Zingerman @ 2026-08-14 20:44 UTC (permalink / raw)
To: Vineet Gupta, bpf; +Cc: ast, daniel, andrii, x86, stable
On Fri, 2026-08-14 at 13:32 -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>
> ---
Reviewed-by: Eduard Zingerman <eddyz87@gmail.com>
Please don't drop acks.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH bpf v2 2/2] selftests/bpf: Check per-CPU address resolution per register
2026-08-14 20:32 ` [PATCH bpf v2 2/2] selftests/bpf: Check per-CPU address resolution per register Vineet Gupta
@ 2026-08-14 20:49 ` Eduard Zingerman
2026-08-14 21:20 ` bot+bpf-ci
1 sibling, 0 replies; 9+ messages in thread
From: Eduard Zingerman @ 2026-08-14 20:49 UTC (permalink / raw)
To: Vineet Gupta, bpf; +Cc: ast, daniel, andrii, x86, stable
On Fri, 2026-08-14 at 13:32 -0700, Vineet Gupta wrote:
...
> +/*
> + * An ld_imm64 of a per-CPU map value is followed by a mov_percpu_addr that
> + * reuses the same register, so the register the address lands in decides how
> + * the JIT encodes the add. On x86 R5, R7, R8 and R9 are the extended
> + * registers, whose high bit needs REX.R because the destination sits in
> + * ModRM.reg. Check one program per extended register, since getting the
> + * prefix wrong resolves the address into whichever register shares the low
> + * three bits instead.
> + */
> +
> +SEC("raw_tp")
> +__description("per-CPU address into r5")
> +__success
> +__arch_x86_64
> +__jited(" addq %gs:{{.*}}, %r8")
> +__naked void percpu_addr_into_r5(void)
> +{
> + asm volatile (" \
> + r5 = %[percpu_data] ll; \
> + r0 = *(u32 *)(r5 + 0); \
> + exit; \
> +" :
> + : __imm_addr(percpu_data)
> + : __clobber_all);
> +}
Thank you for adding the tests.
Maybe save some space by just packing it as a single test?
__jited(" addq %gs:{{.*}}, %...")
__jited(" addq %gs:{{.*}}, %...")
__jited(" addq %gs:{{.*}}, %...")
__jited(" addq %gs:{{.*}}, %...")
__jited(" addq %gs:{{.*}}, %...")
r0 = %[percpu_data] ll; \
r1 = %[percpu_data] ll; \
r2 = %[percpu_data] ll; \
r3 = %[percpu_data] ll; \
r4 = %[percpu_data] ll; \
r5 = %[percpu_data] ll; \
r6 = %[percpu_data] ll; \
r7 = %[percpu_data] ll; \
r8 = %[percpu_data] ll; \
r9 = %[percpu_data] ll; \
More compact and no need to describe in length why only R{5,7,8,9} are selected.
...
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH bpf v2 1/2] bpf, x86: Fix per-CPU address resolution into an extended register
2026-08-14 20:32 ` [PATCH bpf v2 1/2] bpf, x86: Fix " Vineet Gupta
2026-08-14 20:44 ` Eduard Zingerman
@ 2026-08-14 21:20 ` bot+bpf-ci
2026-08-14 21:30 ` Vineet Gupta
1 sibling, 1 reply; 9+ messages in thread
From: bot+bpf-ci @ 2026-08-14 21:20 UTC (permalink / raw)
To: vineet.gupta, bpf
Cc: ast, daniel, andrii, x86, stable, vineet.gupta, ast, andrii,
daniel, martin.lau, eddyz87, yonghong.song, clm, ihor.solodrai
[-- Attachment #1: Type: text/plain, Size: 3949 bytes --]
> 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: stable@vger.kernel.org
> Signed-off-by: Vineet Gupta <vineet.gupta@linux.dev>
Should this include Eduard Zingerman's Reviewed-by tag? In the v2 review
thread, Eduard provided "Reviewed-by: Eduard Zingerman <eddyz87@gmail.com>"
and specifically noted "Please don't drop acks."
https://lore.kernel.org/bpf/3338fd93155ac14b3b824ce232ae7be288072608.camel@gmail.com/
> 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/5acb321999d7cc7ebd945c519495a8762501e3cc.camel@gmail.com/
---
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH bpf v2 2/2] selftests/bpf: Check per-CPU address resolution per register
2026-08-14 20:32 ` [PATCH bpf v2 2/2] selftests/bpf: Check per-CPU address resolution per register Vineet Gupta
2026-08-14 20:49 ` Eduard Zingerman
@ 2026-08-14 21:20 ` bot+bpf-ci
2026-08-14 21:51 ` Vineet Gupta
1 sibling, 1 reply; 9+ messages in thread
From: bot+bpf-ci @ 2026-08-14 21:20 UTC (permalink / raw)
To: vineet.gupta, bpf
Cc: ast, daniel, andrii, x86, stable, vineet.gupta, ast, andrii,
daniel, martin.lau, eddyz87, yonghong.song, clm, ihor.solodrai
[-- Attachment #1: Type: text/plain, Size: 2421 bytes --]
> diff --git a/tools/testing/selftests/bpf/progs/verifier_percpu_addr.c b/tools/testing/selftests/bpf/progs/verifier_percpu_addr.c
> new file mode 100644
> index 000000000000..962faea8ef90
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/verifier_percpu_addr.c
> @@ -0,0 +1,114 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <vmlinux.h>
> +#include <bpf/bpf_helpers.h>
> +#include "bpf_misc.h"
> +
> +int percpu_data SEC(".percpu");
> +
> +#if defined(__TARGET_ARCH_x86)
> +
> +/*
> + * An ld_imm64 of a per-CPU map value is followed by a mov_percpu_addr that
> + * reuses the same register, so the register the address lands in decides how
> + * the JIT encodes the add. On x86 R5, R7, R8 and R9 are the extended
> + * registers, whose high bit needs REX.R because the destination sits in
> + * ModRM.reg. Check one program per extended register, since getting the
> + * prefix wrong resolves the address into whichever register shares the low
> + * three bits instead.
> + */
> +
> +SEC("raw_tp")
> +__description("per-CPU address into r5")
> +__success
> +__arch_x86_64
> +__jited(" addq %gs:{{.*}}, %r8")
> +__naked void percpu_addr_into_r5(void)
> +{
> + asm volatile (" \
> + r5 = %[percpu_data] ll; \
> + r0 = *(u32 *)(r5 + 0); \
> + exit; \
> +" :
> + : __imm_addr(percpu_data)
> + : __clobber_all);
> +}
This isn't a bug, but would it be tidier to move the percpu_data
declaration inside the __TARGET_ARCH_x86 block?
Currently percpu_data is declared above the arch guard, but its only
readers (percpu_addr_into_r1/r5/r7/r8/r9) are inside the guard:
+int percpu_data SEC(".percpu");
+
+#if defined(__TARGET_ARCH_x86)
+
+/* ... test functions using percpu_data ... */
On every other target the object still carries a .percpu section, so
libbpf still creates an unused percpu data map for it.
Comparable files in the same directory keep objects used only by guarded
programs inside the guard. verifier_private_stack.c declares its 'struct
elem' and 'array SEC(".maps")' after the '#if defined(__TARGET_ARCH_x86)
|| defined(__TARGET_ARCH_arm64)' guard, and verifier_ldsx.c declares its
'arena SEC(".maps")' map inside its arch guard.
---
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH bpf v2 1/2] bpf, x86: Fix per-CPU address resolution into an extended register
2026-08-14 21:20 ` bot+bpf-ci
@ 2026-08-14 21:30 ` Vineet Gupta
0 siblings, 0 replies; 9+ messages in thread
From: Vineet Gupta @ 2026-08-14 21:30 UTC (permalink / raw)
To: bot+bpf-ci, bpf
Cc: ast, daniel, andrii, x86, stable, martin.lau, eddyz87,
yonghong.song, clm, ihor.solodrai
On 8/14/26 2:20 PM, bot+bpf-ci@kernel.org 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: stable@vger.kernel.org
>> Signed-off-by: Vineet Gupta <vineet.gupta@linux.dev>
> Should this include Eduard Zingerman's Reviewed-by tag? In the v2 review
> thread, Eduard provided "Reviewed-by: Eduard Zingerman <eddyz87@gmail.com>"
> and specifically noted "Please don't drop acks."
>
> https://lore.kernel.org/bpf/3338fd93155ac14b3b824ce232ae7be288072608.camel@gmail.com/
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/5acb321999d7cc7ebd945c519495a8762501e3cc.camel@gmail.com/
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH bpf v2 2/2] selftests/bpf: Check per-CPU address resolution per register
2026-08-14 21:20 ` bot+bpf-ci
@ 2026-08-14 21:51 ` Vineet Gupta
0 siblings, 0 replies; 9+ messages in thread
From: Vineet Gupta @ 2026-08-14 21:51 UTC (permalink / raw)
To: bot+bpf-ci, bpf
Cc: ast, daniel, andrii, x86, stable, martin.lau, eddyz87,
yonghong.song, clm, ihor.solodrai
On 8/14/26 2:20 PM, bot+bpf-ci@kernel.org wrote:
>> diff --git a/tools/testing/selftests/bpf/progs/verifier_percpu_addr.c b/tools/testing/selftests/bpf/progs/verifier_percpu_addr.c
>> new file mode 100644
>> index 000000000000..962faea8ef90
>> --- /dev/null
>> +++ b/tools/testing/selftests/bpf/progs/verifier_percpu_addr.c
>> @@ -0,0 +1,114 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +
>> +#include <vmlinux.h>
>> +#include <bpf/bpf_helpers.h>
>> +#include "bpf_misc.h"
>> +
>> +int percpu_data SEC(".percpu");
>> +
>> +#if defined(__TARGET_ARCH_x86)
>> +
>> +/*
>> + * An ld_imm64 of a per-CPU map value is followed by a mov_percpu_addr that
>> + * reuses the same register, so the register the address lands in decides how
>> + * the JIT encodes the add. On x86 R5, R7, R8 and R9 are the extended
>> + * registers, whose high bit needs REX.R because the destination sits in
>> + * ModRM.reg. Check one program per extended register, since getting the
>> + * prefix wrong resolves the address into whichever register shares the low
>> + * three bits instead.
>> + */
>> +
>> +SEC("raw_tp")
>> +__description("per-CPU address into r5")
>> +__success
>> +__arch_x86_64
>> +__jited(" addq %gs:{{.*}}, %r8")
>> +__naked void percpu_addr_into_r5(void)
>> +{
>> + asm volatile (" \
>> + r5 = %[percpu_data] ll; \
>> + r0 = *(u32 *)(r5 + 0); \
>> + exit; \
>> +" :
>> + : __imm_addr(percpu_data)
>> + : __clobber_all);
>> +}
> This isn't a bug, but would it be tidier to move the percpu_data
> declaration inside the __TARGET_ARCH_x86 block?
>
> Currently percpu_data is declared above the arch guard, but its only
> readers (percpu_addr_into_r1/r5/r7/r8/r9) are inside the guard:
>
> +int percpu_data SEC(".percpu");
> +
> +#if defined(__TARGET_ARCH_x86)
> +
> +/* ... test functions using percpu_data ... */
>
> On every other target the object still carries a .percpu section, so
> libbpf still creates an unused percpu data map for it.
That's a good point, will address this in v3
Thx,
-Vineet
>
> Comparable files in the same directory keep objects used only by guarded
> programs inside the guard. verifier_private_stack.c declares its 'struct
> elem' and 'array SEC(".maps")' after the '#if defined(__TARGET_ARCH_x86)
> || defined(__TARGET_ARCH_arm64)' guard, and verifier_ldsx.c declares its
> 'arena SEC(".maps")' map inside its arch guard.
>
>
> ---
> 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
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2026-08-14 21:52 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-14 20:32 [PATCH bpf v2 0/2] bpf, x86: fix per-CPU address resolution into an extended register Vineet Gupta
2026-08-14 20:32 ` [PATCH bpf v2 1/2] bpf, x86: Fix " Vineet Gupta
2026-08-14 20:44 ` Eduard Zingerman
2026-08-14 21:20 ` bot+bpf-ci
2026-08-14 21:30 ` Vineet Gupta
2026-08-14 20:32 ` [PATCH bpf v2 2/2] selftests/bpf: Check per-CPU address resolution per register Vineet Gupta
2026-08-14 20:49 ` Eduard Zingerman
2026-08-14 21:20 ` bot+bpf-ci
2026-08-14 21:51 ` Vineet Gupta
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox