* [PATCH bpf-next v2 1/2] bpf: Use tnums for JEQ/JNE is_branch_taken logic
@ 2025-08-20 13:18 Paul Chaignon
2025-08-20 13:19 ` [PATCH bpf-next v2 2/2] selftests/bpf: Tests for is_scalar_branch_taken tnum logic Paul Chaignon
2025-08-22 16:20 ` [PATCH bpf-next v2 1/2] bpf: Use tnums for JEQ/JNE is_branch_taken logic patchwork-bot+netdevbpf
0 siblings, 2 replies; 3+ messages in thread
From: Paul Chaignon @ 2025-08-20 13:18 UTC (permalink / raw)
To: bpf
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Eduard Zingerman, Yonghong Song, Shung-Hsi Yu
In the following toy program (reg states minimized for readability), R0
and R1 always have different values at instruction 6. This is obvious
when reading the program but cannot be guessed from ranges alone as
they overlap (R0 in [0; 0xc0000000], R1 in [1024; 0xc0000400]).
0: call bpf_get_prandom_u32#7 ; R0_w=scalar()
1: w0 = w0 ; R0_w=scalar(var_off=(0x0; 0xffffffff))
2: r0 >>= 30 ; R0_w=scalar(var_off=(0x0; 0x3))
3: r0 <<= 30 ; R0_w=scalar(var_off=(0x0; 0xc0000000))
4: r1 = r0 ; R1_w=scalar(var_off=(0x0; 0xc0000000))
5: r1 += 1024 ; R1_w=scalar(var_off=(0x400; 0xc0000000))
6: if r1 != r0 goto pc+1
Looking at tnums however, we can deduce that R1 is always different from
R0 because their tnums don't agree on known bits. This patch uses this
logic to improve is_scalar_branch_taken in case of BPF_JEQ and BPF_JNE.
This change has a tiny impact on complexity, which was measured with
the Cilium complexity CI test. That test covers 72 programs with
various build and load time configurations for a total of 970 test
cases. For 80% of test cases, the patch has no impact. On the other
test cases, the patch decreases complexity by only 0.08% on average. In
the best case, the verifier needs to walk 3% less instructions and, in
the worst case, 1.5% more. Overall, the patch has a small positive
impact, especially for our largest programs.
Acked-by: Eduard Zingerman <eddyz87@gmail.com>
Acked-by: Shung-Hsi Yu <shung-hsi.yu@suse.com>
Signed-off-by: Paul Chaignon <paul.chaignon@gmail.com>
---
Changes in v2:
- Renamed tnum function to tnum_overlap as suggested by Shung-Hsi.
- Grouped tnum checks together in is_scalar_branch_taken, as
suggested by Shung-Hsi.
- Rebased.
include/linux/tnum.h | 3 +++
kernel/bpf/tnum.c | 8 ++++++++
kernel/bpf/verifier.c | 4 ++++
3 files changed, 15 insertions(+)
diff --git a/include/linux/tnum.h b/include/linux/tnum.h
index 57ed3035cc30..0ffb77ffe0e8 100644
--- a/include/linux/tnum.h
+++ b/include/linux/tnum.h
@@ -51,6 +51,9 @@ struct tnum tnum_xor(struct tnum a, struct tnum b);
/* Multiply two tnums, return @a * @b */
struct tnum tnum_mul(struct tnum a, struct tnum b);
+/* Return true if the known bits of both tnums have the same value */
+bool tnum_overlap(struct tnum a, struct tnum b);
+
/* Return a tnum representing numbers satisfying both @a and @b */
struct tnum tnum_intersect(struct tnum a, struct tnum b);
diff --git a/kernel/bpf/tnum.c b/kernel/bpf/tnum.c
index fa353c5d550f..d9328bbb3680 100644
--- a/kernel/bpf/tnum.c
+++ b/kernel/bpf/tnum.c
@@ -143,6 +143,14 @@ struct tnum tnum_mul(struct tnum a, struct tnum b)
return tnum_add(TNUM(acc_v, 0), acc_m);
}
+bool tnum_overlap(struct tnum a, struct tnum b)
+{
+ u64 mu;
+
+ mu = ~a.mask & ~b.mask;
+ return (a.value & mu) == (b.value & mu);
+}
+
/* Note that if a and b disagree - i.e. one has a 'known 1' where the other has
* a 'known 0' - this will return a 'known 1' for that bit.
*/
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 4e47992361ea..5c9dd16b2c56 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -15897,6 +15897,8 @@ static int is_scalar_branch_taken(struct bpf_reg_state *reg1, struct bpf_reg_sta
*/
if (tnum_is_const(t1) && tnum_is_const(t2))
return t1.value == t2.value;
+ if (!tnum_overlap(t1, t2))
+ return 0;
/* non-overlapping ranges */
if (umin1 > umax2 || umax1 < umin2)
return 0;
@@ -15921,6 +15923,8 @@ static int is_scalar_branch_taken(struct bpf_reg_state *reg1, struct bpf_reg_sta
*/
if (tnum_is_const(t1) && tnum_is_const(t2))
return t1.value != t2.value;
+ if (!tnum_overlap(t1, t2))
+ return 1;
/* non-overlapping ranges */
if (umin1 > umax2 || umax1 < umin2)
return 1;
--
2.43.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [PATCH bpf-next v2 2/2] selftests/bpf: Tests for is_scalar_branch_taken tnum logic
2025-08-20 13:18 [PATCH bpf-next v2 1/2] bpf: Use tnums for JEQ/JNE is_branch_taken logic Paul Chaignon
@ 2025-08-20 13:19 ` Paul Chaignon
2025-08-22 16:20 ` [PATCH bpf-next v2 1/2] bpf: Use tnums for JEQ/JNE is_branch_taken logic patchwork-bot+netdevbpf
1 sibling, 0 replies; 3+ messages in thread
From: Paul Chaignon @ 2025-08-20 13:19 UTC (permalink / raw)
To: bpf
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Eduard Zingerman, Yonghong Song, Shung-Hsi Yu
This patch adds tests for the new jeq and jne logic in
is_scalar_branch_taken. The following shows the first test failing
before the previous patch is applied. Once the previous patch is
applied, the verifier can use the tnum values to deduce that instruction
7 is dead code.
0: call bpf_get_prandom_u32#7 ; R0_w=scalar()
1: w0 = w0 ; R0_w=scalar(smin=0,smax=umax=0xffffffff,var_off=(0x0; 0xffffffff))
2: r0 >>= 30 ; R0_w=scalar(smin=smin32=0,smax=umax=smax32=umax32=3,var_off=(0x0; 0x3))
3: r0 <<= 30 ; R0_w=scalar(smin=0,smax=umax=umax32=0xc0000000,smax32=0x40000000,var_off=(0x0; 0xc0000000))
4: r1 = r0 ; R0_w=scalar(id=1,smin=0,smax=umax=umax32=0xc0000000,smax32=0x40000000,var_off=(0x0; 0xc0000000)) R1_w=scalar(id=1,smin=0,smax=umax=umax32=0xc0000000,smax32=0x40000000,var_off=(0x0; 0xc0000000))
5: r1 += 1024 ; R1_w=scalar(smin=umin=umin32=1024,smax=umax=umax32=0xc0000400,smin32=0x80000400,smax32=0x40000400,var_off=(0x400; 0xc0000000))
6: if r1 != r0 goto pc+1 ; R0_w=scalar(id=1,smin=umin=umin32=1024,smax=umax=umax32=0xc0000000,smin32=0x80000400,smax32=0x40000000,var_off=(0x400; 0xc0000000)) R1_w=scalar(smin=umin=umin32=1024,smax=umax=umax32=0xc0000000,smin32=0x80000400,smax32=0x40000400,var_off=(0x400; 0xc0000000))
7: r10 = 0
frame pointer is read only
Acked-by: Eduard Zingerman <eddyz87@gmail.com>
Signed-off-by: Paul Chaignon <paul.chaignon@gmail.com>
---
Changes in v2:
- Rebased.
.../selftests/bpf/progs/verifier_bounds.c | 41 +++++++++++++++++++
1 file changed, 41 insertions(+)
diff --git a/tools/testing/selftests/bpf/progs/verifier_bounds.c b/tools/testing/selftests/bpf/progs/verifier_bounds.c
index 87a2c60d86e6..fbccc20555f4 100644
--- a/tools/testing/selftests/bpf/progs/verifier_bounds.c
+++ b/tools/testing/selftests/bpf/progs/verifier_bounds.c
@@ -1668,4 +1668,45 @@ l0_%=: r0 = 0; \
: __clobber_all);
}
+SEC("socket")
+__description("dead jne branch due to disagreeing tnums")
+__success __log_level(2)
+__naked void jne_disagreeing_tnums(void *ctx)
+{
+ asm volatile(" \
+ call %[bpf_get_prandom_u32]; \
+ w0 = w0; \
+ r0 >>= 30; \
+ r0 <<= 30; \
+ r1 = r0; \
+ r1 += 1024; \
+ if r1 != r0 goto +1; \
+ r10 = 0; \
+ exit; \
+" :
+ : __imm(bpf_get_prandom_u32)
+ : __clobber_all);
+}
+
+SEC("socket")
+__description("dead jeq branch due to disagreeing tnums")
+__success __log_level(2)
+__naked void jeq_disagreeing_tnums(void *ctx)
+{
+ asm volatile(" \
+ call %[bpf_get_prandom_u32]; \
+ w0 = w0; \
+ r0 >>= 30; \
+ r0 <<= 30; \
+ r1 = r0; \
+ r1 += 1024; \
+ if r1 == r0 goto +1; \
+ exit; \
+ r10 = 0; \
+ exit; \
+" :
+ : __imm(bpf_get_prandom_u32)
+ : __clobber_all);
+}
+
char _license[] SEC("license") = "GPL";
--
2.43.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH bpf-next v2 1/2] bpf: Use tnums for JEQ/JNE is_branch_taken logic
2025-08-20 13:18 [PATCH bpf-next v2 1/2] bpf: Use tnums for JEQ/JNE is_branch_taken logic Paul Chaignon
2025-08-20 13:19 ` [PATCH bpf-next v2 2/2] selftests/bpf: Tests for is_scalar_branch_taken tnum logic Paul Chaignon
@ 2025-08-22 16:20 ` patchwork-bot+netdevbpf
1 sibling, 0 replies; 3+ messages in thread
From: patchwork-bot+netdevbpf @ 2025-08-22 16:20 UTC (permalink / raw)
To: Paul Chaignon
Cc: bpf, ast, daniel, andrii, eddyz87, yonghong.song, shung-hsi.yu
Hello:
This series was applied to bpf/bpf-next.git (master)
by Daniel Borkmann <daniel@iogearbox.net>:
On Wed, 20 Aug 2025 15:18:06 +0200 you wrote:
> In the following toy program (reg states minimized for readability), R0
> and R1 always have different values at instruction 6. This is obvious
> when reading the program but cannot be guessed from ranges alone as
> they overlap (R0 in [0; 0xc0000000], R1 in [1024; 0xc0000400]).
>
> 0: call bpf_get_prandom_u32#7 ; R0_w=scalar()
> 1: w0 = w0 ; R0_w=scalar(var_off=(0x0; 0xffffffff))
> 2: r0 >>= 30 ; R0_w=scalar(var_off=(0x0; 0x3))
> 3: r0 <<= 30 ; R0_w=scalar(var_off=(0x0; 0xc0000000))
> 4: r1 = r0 ; R1_w=scalar(var_off=(0x0; 0xc0000000))
> 5: r1 += 1024 ; R1_w=scalar(var_off=(0x400; 0xc0000000))
> 6: if r1 != r0 goto pc+1
>
> [...]
Here is the summary with links:
- [bpf-next,v2,1/2] bpf: Use tnums for JEQ/JNE is_branch_taken logic
https://git.kernel.org/bpf/bpf-next/c/f41345f47fb2
- [bpf-next,v2,2/2] selftests/bpf: Tests for is_scalar_branch_taken tnum logic
https://git.kernel.org/bpf/bpf-next/c/0780f54ab129
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-08-22 16:20 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-20 13:18 [PATCH bpf-next v2 1/2] bpf: Use tnums for JEQ/JNE is_branch_taken logic Paul Chaignon
2025-08-20 13:19 ` [PATCH bpf-next v2 2/2] selftests/bpf: Tests for is_scalar_branch_taken tnum logic Paul Chaignon
2025-08-22 16:20 ` [PATCH bpf-next v2 1/2] bpf: Use tnums for JEQ/JNE is_branch_taken logic patchwork-bot+netdevbpf
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).