All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eduard Zingerman <eddyz87@gmail.com>
To: Yonghong Song <yonghong.song@linux.dev>, bpf@vger.kernel.org
Cc: Alexei Starovoitov <ast@kernel.org>,
	Andrii Nakryiko <andrii@kernel.org>,
	 Daniel Borkmann <daniel@iogearbox.net>,
	kernel-team@fb.com
Subject: Re: [PATCH bpf-next] bpf: Track linked scalars across a "rX <<= 32; rX >>= 32" zero extension
Date: Thu, 20 Aug 2026 09:57:33 -0700	[thread overview]
Message-ID: <88b2bba531714814df5a2c4fe9d10aadfd4cce47.camel@gmail.com> (raw)
In-Reply-To: <20260820013925.2515018-1-yonghong.song@linux.dev>

On Wed, 2026-08-19 at 18:39 -0700, Yonghong Song wrote:
> For the following test in
> tools/testing/selftests/bpf/progs/verifier_linked_scalars.c:
> 
> 	void alu32_negative_offset(void)
> 	{
> 		volatile char path[5];
> 		volatile int offset = bpf_get_prandom_u32();
> 		int off = offset;
> 
> 		if (off >= 5 && off < 10)
> 			path[off - 5] = '.';
> 
> 		/* So compiler doesn't say: error: variable 'path' set but not used */
> 		__sink(path[0]);
> 	}
> 
> Without alu32 (-mcpu=v2), the test
> verifier_linked_scalars/alu32_negative_offset will fail with llvm22 and
> llvm23 like below.
> 
>   3: (bf) r2 = r1               ; R1=scalar(id=1,...) R2=scalar(id=1,...)
>   4: (07) r2 += -5              ; R2=scalar(id=1-5,smin=-5,smax=0xfffffffa)
>   5: (67) r2 <<= 32             ; R2=scalar(smax=0x7fffffff00000000,...)
>   6: (77) r2 >>= 32             ; R2=scalar(smin=0,umax=0xffffffff,...)
>   7: (25) if r2 > 0x4 goto pc+5 ; R2=scalar(smin=0,smax=umax=4,...)
>   8: (bf) r2 = r10
>   9: (07) r2 += -5
>  10: (0f) r2 += r1              ; R1=scalar(id=1,smin=0,umax=0xffffffff)
>                                 ; R2=fp(smin=-5,smax=0xfffffffa)
>  11: (b7) r1 = 46               ; R1=46
>  12: (73) *(u8 *)(r2 -5) = r1
>  invalid unbounded variable-offset write to stack R2
> 
> R1 is never narrowed down, so the address stays unbounded and the store
> is rejected.
> 
> The test is okay for llvm21 with -mcpu=v2, see below:
> 
>   3: (07) r1 += -5              ; R1=scalar(smin=-5,smax=0xfffffffa)
>   4: (67) r1 <<= 32             ; R1=scalar(smax=0x7fffffff00000000,...)
>   5: (77) r1 >>= 32             ; R1=scalar(smin=0,umax=0xffffffff,...)
>   6: (25) if r1 > 0x4 goto pc+5 ; R1=scalar(smin=0,smax=umax=4,...)
>   7: (bf) r2 = r10
>   8: (07) r2 += -5
>   9: (0f) r2 += r1              ; R2=fp(smin=-5,smax=-1)
>  10: (b7) r1 = 46               ; R1=46
>  11: (73) *(u8 *)(r2 +0) = r1   ; fp-8=ppppm???
> 
> To fix the test issue with llvm22 and llvm23, note that the shift pair
> computes zext32(base + delta), which is exactly the relation
> BPF_ADD_CONST32 describes. So keep the link alive across the first
> shift and turn it from a 64-bit into a 32-bit one at the second, which
> makes the -mcpu=v2 sequence track like an alu32 one.
> 
> Two conditions guard this. First, a 32-bit link requires the linked
> value to fit into u32, because sync_linked_regs() zero extends the
> bounds it propagates through such a link. linked_base_fits_u32() checks
> that on the register state before the shift, mirroring the dst_umax
> check the alu32 add path already does. Second, in between the two
> shifts the register does not hold the value its id and delta describe,
> so the second shift must have a single incoming edge, otherwise the
> intermediate state could be checkpointed and another path pruned
> against it.
> 
> With this, the llvm22 and llvm23 code verifies, R2 keeps its id through
> both shifts and the jump narrows down R1:
> 
>   3: (bf) r2 = r1               ; R1=scalar(id=1,...) R2=scalar(id=1,...)
>   4: (07) r2 += -5              ; R2=scalar(id=1-5,smin=-5,smax=0xfffffffa)
>   5: (67) r2 <<= 32             ; R2=scalar(id=1-5,smax=0x7fffffff00000000)
>   6: (77) r2 >>= 32             ; R2=scalar(id=1-5,smin=0,umax=0xffffffff)
>   7: (25) if r2 > 0x4 goto pc+5 ; R1=scalar(id=1,smin=5,smax=9,...)
>                                 ; R2=scalar(id=1-5,smin=0,smax=4,...)
>   8: (bf) r2 = r10
>   9: (07) r2 += -5
>  10: (0f) r2 += r1              ; R2=fp(smin=0,smax=4)
>  11: (b7) r1 = 46               ; R1=46
>  12: (73) *(u8 *)(r2 -5) = r1   ; fp-8=ppppm???
> 
> The llvm21 log is unchanged, R1 carries no id there so the new code
> does not apply to it.
> 
> Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
> ---

I think this is too tricky. A simpler thing is to rewrite the incoming
program as `w2 = w1; nop;` in place of two shifts. The CFG check would
still be necessary, though.

...

  parent reply	other threads:[~2026-08-20 16:57 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-20  1:39 [PATCH bpf-next] bpf: Track linked scalars across a "rX <<= 32; rX >>= 32" zero extension Yonghong Song
2026-08-20  2:33 ` bot+bpf-ci
2026-08-20 16:57 ` Eduard Zingerman [this message]
2026-08-20 18:55   ` Yonghong Song

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=88b2bba531714814df5a2c4fe9d10aadfd4cce47.camel@gmail.com \
    --to=eddyz87@gmail.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=kernel-team@fb.com \
    --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 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.