From: Vineet Gupta <vineet.gupta@linux.dev>
To: bot+bpf-ci@kernel.org, bpf@vger.kernel.org
Cc: ast@kernel.org, daniel@iogearbox.net, andrii@kernel.org,
x86@kernel.org, stable@vger.kernel.org, martin.lau@kernel.org,
eddyz87@gmail.com, yonghong.song@linux.dev, clm@meta.com,
ihor.solodrai@linux.dev
Subject: Re: [PATCH bpf v3 2/2] selftests/bpf: Check per-CPU address resolution per register
Date: Fri, 14 Aug 2026 16:55:00 -0700 [thread overview]
Message-ID: <fc9da1e4-714a-4a2a-974b-fe495a3d8afe@linux.dev> (raw)
In-Reply-To: <67663b343f4aa2719eee273f10ca74a469fa1ab5e8772933083f51b62284a59a@mail.kernel.org>
On 8/14/26 3:39 PM, bot+bpf-ci@kernel.org wrote:
>> commit fe4e8b7aecfff8c04bc3c2f472f9243928effc6e
>> Author: Vineet Gupta <vineet.gupta@linux.dev>
>> Date: Fri Aug 14 15:02:54 2026 -0700
>>
>> selftests/bpf: Check per-CPU address resolution per register
>>
>> 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 unless the address happens to land in an
>> extended register, which is why this went unnoticed.
> Does this mechanism actually exist in the tree?
Indeed these changes are introduced in bpf-next not on *bpf* which the
patches were targeting - noob mistake on my part.
I'll respin v4 targeting bpf-next and drop stable etc.
Thx,
-Vineet
>
> There is no verifier or JIT path that follows an ld_imm64 of a per-CPU map
> value with a mov_percpu_addr on the same register. BPF_MOV64_PERCPU_REG has
> exactly four call sites:
>
> kernel/bpf/arraymap.c:288 *insn++ = BPF_MOV64_PERCPU_REG(BPF_REG_0, BPF_REG_0);
> kernel/bpf/hashtab.c:2429 *insn++ = BPF_MOV64_PERCPU_REG(BPF_REG_0, BPF_REG_0);
> kernel/bpf/fixups.c:2115 insn_buf[1] = BPF_MOV64_PERCPU_REG(BPF_REG_0, BPF_REG_0);
> kernel/bpf/fixups.c:2136 insn_buf[1] = BPF_MOV64_PERCPU_REG(BPF_REG_0, BPF_REG_0);
>
> All four use BPF_REG_0 as both source and destination. BPF_REG_0 maps to
> RAX, so is_ereg() is always false and the REX.R encoding path is
> unreachable from any loadable program.
>
> An ld_imm64 of a percpu ksym goes through check_pseudo_btf_id() in the
> verifier, which only sets aux->btf_var.reg_type = PTR_TO_BTF_ID |
> MEM_PERCPU. No mov_percpu_addr insn is inserted after it. The per-CPU
> base is added later by bpf_per_cpu_ptr() or bpf_this_cpu_ptr() kfuncs,
> which return in R0.
>
>> 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..967f4e6e3a49
>> --- /dev/null
>> +++ b/tools/testing/selftests/bpf/progs/verifier_percpu_addr.c
>> @@ -0,0 +1,72 @@
> [ ... ]
>
>> +SEC("raw_tp")
>> +__description("per-CPU address resolution")
>> +__success
>> +__arch_x86_64
>> +__jited(" movabsq $0x{{.*}}, %rax")
>> +__jited(" addq %gs:{{.*}}, %rax")
>> +__jited(" movabsq $0x{{.*}}, %rdi")
>> +__jited(" addq %gs:{{.*}}, %rdi")
>> +__jited(" movabsq $0x{{.*}}, %rsi")
>> +__jited(" addq %gs:{{.*}}, %rsi")
>> +__jited(" movabsq $0x{{.*}}, %rdx")
>> +__jited(" addq %gs:{{.*}}, %rdx")
>> +__jited(" movabsq $0x{{.*}}, %rcx")
>> +__jited(" addq %gs:{{.*}}, %rcx")
>> +__jited(" movabsq $0x{{.*}}, %r8")
>> +__jited(" addq %gs:{{.*}}, %r8")
>> +__jited(" movabsq $0x{{.*}}, %rbx")
>> +__jited(" addq %gs:{{.*}}, %rbx")
>> +__jited(" movabsq $0x{{.*}}, %r13")
>> +__jited(" addq %gs:{{.*}}, %r13")
>> +__jited(" movabsq $0x{{.*}}, %r14")
>> +__jited(" addq %gs:{{.*}}, %r14")
>> +__jited(" movabsq $0x{{.*}}, %r15")
>> +__jited(" addq %gs:{{.*}}, %r15")
>> +__naked void percpu_addr(void)
>> +{
>> + asm volatile (" \
>> + 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; \
>> + r0 = 0; \
>> + exit; \
>> +" :
>> + : __imm_addr(percpu_data)
>> + : __clobber_all);
>> +}
> Can these 'addq %gs:..., %<reg>' assertions ever match?
>
> A bare sequence of ten ld_imm64s produces ten 'movabsq' lines and zero
> 'addq %gs:' lines. The verifier does not insert a mov_percpu_addr insn
> after an ld_imm64 of a percpu ksym. The per-CPU base is added by the
> bpf_per_cpu_ptr() or bpf_this_cpu_ptr() kfuncs, which return in R0.
>
> So the JIT output would be ten consecutive 'movabsq' instructions with no
> 'addq' between them. The test expects alternating 'movabsq' and 'addq'
> lines, and consecutive __jited tags must match consecutive disassembly
> lines unless a '...' tag is inserted.
>
> Even if a mov_percpu_addr could be generated, all four call sites hardcode
> BPF_REG_0, so only the %rax form would ever be reachable.
>
> This means the test cannot cover the REX.R fix in acdf39f8be46 ("bpf, x86:
> Fix per-CPU address resolution into an extended register").
>
> Should there be a Link: or Fixes: tag referencing acdf39f8be46, and does
> the commit message need to explain what new path makes the destination
> register reachable?
>
>
> ---
> 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/31845422431
next prev parent reply other threads:[~2026-08-14 23:55 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-08-14 22:02 [PATCH bpf v3 0/2] bpf, x86: fix per-CPU address resolution into an extended register Vineet Gupta
2026-08-14 22:02 ` [PATCH bpf v3 1/2] bpf, x86: Fix " Vineet Gupta
2026-08-14 22:02 ` [PATCH bpf v3 2/2] selftests/bpf: Check per-CPU address resolution per register Vineet Gupta
2026-08-14 22:39 ` bot+bpf-ci
2026-08-14 23:55 ` Vineet Gupta [this message]
2026-08-15 1:12 ` Borislav Petkov
2026-08-15 6:59 ` [PATCH bpf v3 0/2] bpf, x86: fix per-CPU address resolution into an extended register Eduard Zingerman
2026-08-15 7:00 ` patchwork-bot+netdevbpf
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=fc9da1e4-714a-4a2a-974b-fe495a3d8afe@linux.dev \
--to=vineet.gupta@linux.dev \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bot+bpf-ci@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=clm@meta.com \
--cc=daniel@iogearbox.net \
--cc=eddyz87@gmail.com \
--cc=ihor.solodrai@linux.dev \
--cc=martin.lau@kernel.org \
--cc=stable@vger.kernel.org \
--cc=x86@kernel.org \
--cc=yonghong.song@linux.dev \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox