Linux kernel -stable discussions
 help / color / mirror / Atom feed
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


  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