public inbox for bpf@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH bpf-next 0/2] bpf: range_within() must check cnum ranges instead of min/max pairs
@ 2026-04-25 22:48 Eduard Zingerman
  2026-04-25 22:48 ` [PATCH bpf-next 1/2] " Eduard Zingerman
  2026-04-25 22:48 ` [PATCH bpf-next 2/2] selftests/bpf: a test for proper cnums compare in is_state_visited() Eduard Zingerman
  0 siblings, 2 replies; 5+ messages in thread
From: Eduard Zingerman @ 2026-04-25 22:48 UTC (permalink / raw)
  To: bpf, ast, andrii; +Cc: daniel, martin.lau, kernel-team, yonghong.song, eddyz87

This is a follow-up for series [1].
is_state_visited() should check cnum's subset relations using
cnum-based operations, not min/max projections. See patch #1 for
detailed explanation and patch #2 for an example of buggy program
accepted by verifier w/o this fix.

Updated veristat performance numbers compared to master before [1]
merge follow. Measurements done for the same set of selftests/
sched_ext/meta/cilium programs as in [1].

Net increase:   98K insn 88 programs
Net decrease: -282K insn 52 programs

Raw stats filtered as -f insns_pct>1 -f !insns<10000:

========= selftests: master vs experiment =========

File  Program  Insns (A)  Insns (B)  Insns (DIFF)
----  -------  ---------  ---------  ------------

Total progs: 4665
total_insns diff min:   -0.44%
total_insns diff max:   52.94%
total_insns abs max old: 837,487
total_insns abs max new: 837,487
  -5 .. 0    %: 8
   0 .. 5    %: 4652
   5 .. 15   %: 1
  40 .. 50   %: 3
  50 .. 55   %: 1

========= scx: master vs experiment =========

File               Program          Insns (A)  Insns (B)  Insns     (DIFF)
-----------------  ---------------  ---------  ---------  ----------------
scx_layered.bpf.o  layered_enqueue      13718      14402     +684 (+4.99%)
scx_rusty.bpf.o    rusty_enqueue        39842      22053  -17789 (-44.65%)
scx_rusty.bpf.o    rusty_stopping       37738      19949  -17789 (-47.14%)
scx_wd40.bpf.o     wd40_stopping        37729      19880  -17849 (-47.31%)

Total progs: 376
total_insns diff min:  -47.31%
total_insns diff max:   19.61%
total_insns abs max old: 233,669
total_insns abs max new: 233,696
 -50 .. -40  %: 3
  -5 .. 0    %: 3
   0 .. 5    %: 367
   5 .. 15   %: 2
  15 .. 20   %: 1

========= meta: master vs experiment =========

File                                    Program              Insns (A)  Insns (B)  Insns     (DIFF)
--------------------------------------  -------------------  ---------  ---------  ----------------
<sandbox>                               test_file_open           88771     104160  +15389 (+17.34%)
<profiler>                              on_perf_event            48056      54544   +6488 (+13.50%)
<profiler>                              on_tracepoint_event      48059      54547   +6488 (+13.50%)
<profiler>                              on_perf_event            48056      54544   +6488 (+13.50%)
<profiler>                              on_tracepoint_event      48059      54547   +6488 (+13.50%)
<profiler>                              on_alloc                 50445      56933   +6488 (+12.86%)
<profiler>                              on_free                  50251      56739   +6488 (+12.91%)
<profiler>                              on_perf_event            48056      54544   +6488 (+13.50%)
<profiler>                              on_tracepoint_event      48059      54547   +6488 (+13.50%)
<profiler>                              future_iter_resume       50114      56602   +6488 (+12.95%)
<profiler>                              on_py_event              50042      56530   +6488 (+12.97%)
scx_layered_bpf_skel_genskel-bpf.bpf.o  layered_enqueue          13287      13963     +676 (+5.09%)
scx_layered_bpf_skel_genskel-bpf.bpf.o  layered_enqueue          13269      13945     +676 (+5.09%)
scx_layered_bpf_skel_genskel-bpf.bpf.o  layered_enqueue          13269      13945     +676 (+5.09%)
<firewall>                              ..._egress              222327     164648  -57679 (-25.94%)
<firewall>                              ..._tc_eg               222839     164772  -58067 (-26.06%)
<firewall>                              ..._egress              222327     164648  -57679 (-25.94%)
<firewall>                              ..._tc_eg               222839     164772  -58067 (-26.06%)

Total progs: 1540
total_insns diff min:  -26.06%
total_insns diff max:   37.25%
total_insns abs max old: 666,036
total_insns abs max new: 666,036
 -30 .. -20  %: 4
  -5 .. 0    %: 10
   0 .. 5    %: 1494
   5 .. 10   %: 10
  10 .. 15   %: 13
  15 .. 25   %: 5
  35 .. 40   %: 4

========= cilium: master vs experiment =========

File             Program                            Insns (A)  Insns (B)  Insns    (DIFF)
---------------  ---------------------------------  ---------  ---------  ---------------
bpf_host.o       tail_handle_ipv4_cont_from_host        20962      26024  +5062 (+24.15%)
bpf_host.o       tail_handle_ipv6_cont_from_host        17036      18672   +1636 (+9.60%)
bpf_host.o       tail_nodeport_nat_ingress_ipv4         20080      19858    -222 (-1.11%)
bpf_lxc.o        tail_nodeport_nat_ingress_ipv4         10697      10510    -187 (-1.75%)
bpf_overlay.o    tail_handle_inter_cluster_revsnat      11099      10857    -242 (-2.18%)
bpf_overlay.o    tail_nodeport_nat_ingress_ipv4         11951      11768    -183 (-1.53%)
bpf_wireguard.o  tail_nodeport_nat_ingress_ipv4         11993      11811    -182 (-1.52%)

Total progs: 134
total_insns diff min:   -3.32%
total_insns diff max:   24.15%
total_insns abs max old: 152,012
total_insns abs max new: 152,012
  -5 .. 0    %: 12
   0 .. 5    %: 120
   5 .. 15   %: 1
  20 .. 25   %: 1

[1] https://lore.kernel.org/bpf/fd376f47b9512daf669a87b23573f614ec146385.camel@gmail.com/T/

---
Eduard Zingerman (2):
      bpf: range_within() must check cnum ranges instead of min/max pairs
      selftests/bpf: a test for proper cnums compare in is_state_visited()

 include/linux/cnum.h                               |  2 ++
 kernel/bpf/cnum_defs.h                             | 14 +++++++++++
 kernel/bpf/states.c                                | 11 +++------
 .../testing/selftests/bpf/progs/verifier_bounds.c  | 27 ++++++++++++++++++++++
 4 files changed, 46 insertions(+), 8 deletions(-)
---
base-commit: 6c60b2dd5a7889a583389e95e79689191206f86f
change-id: 20260425-cnum-range-within-4a755ba81e93

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH bpf-next 1/2] bpf: range_within() must check cnum ranges instead of min/max pairs
  2026-04-25 22:48 [PATCH bpf-next 0/2] bpf: range_within() must check cnum ranges instead of min/max pairs Eduard Zingerman
@ 2026-04-25 22:48 ` Eduard Zingerman
  2026-04-25 23:20   ` bot+bpf-ci
  2026-04-25 22:48 ` [PATCH bpf-next 2/2] selftests/bpf: a test for proper cnums compare in is_state_visited() Eduard Zingerman
  1 sibling, 1 reply; 5+ messages in thread
From: Eduard Zingerman @ 2026-04-25 22:48 UTC (permalink / raw)
  To: bpf, ast, andrii; +Cc: daniel, martin.lau, kernel-team, yonghong.song, eddyz87

states.c:range_within() must be updated to properly check if
cnum-based range in an old state is a superset of a range in the cur
state. Currently it makes the decision using min/max accessors:

  reg_umin(old) <= reg_umin(cur) <= reg_umax(old)

This is wrong for cnums that cross both UT_MAX/0 and ST_MAX/ST_MIN
boundaries. Consider cnum32{base=0x7FFFFFF0, size=0x80000020},
which represents values [0x7FFFFFF0, ..., U32_MAX, 0, ..., 0x10].
Its projections are u32_min/max=0/U32_MAX, s32_min/max=S32_MIN/MAX.
A register with range [0x100, 0x200] (which lies entirely in the gap
of the wrapping range) would pass the min/max check despite having no
overlap with the actual cnum arc.

This commit replaces min/max comparison with cnum{32,64}_is_subset()
operation. The operation implementation is verified using cbmc model
checker in [1].

[1] https://github.com/eddyz87/cnum-verif/

Fixes: bbc631085503 ("bpf: replace min/max fields with struct cnum{32,64}")
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
---
 include/linux/cnum.h   |  2 ++
 kernel/bpf/cnum_defs.h | 14 ++++++++++++++
 kernel/bpf/states.c    | 11 +++--------
 3 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/include/linux/cnum.h b/include/linux/cnum.h
index a7259b105b45..49b7d0c7645d 100644
--- a/include/linux/cnum.h
+++ b/include/linux/cnum.h
@@ -48,6 +48,7 @@ bool cnum32_is_const(struct cnum32 cnum);
 bool cnum32_is_empty(struct cnum32 cnum);
 struct cnum32 cnum32_add(struct cnum32 a, struct cnum32 b);
 struct cnum32 cnum32_negate(struct cnum32 a);
+bool cnum32_is_subset(struct cnum32 outer, struct cnum32 inner);
 
 /* Same as cnum32 but for 64-bit ranges */
 struct cnum64 {
@@ -73,6 +74,7 @@ bool cnum64_is_const(struct cnum64 cnum);
 bool cnum64_is_empty(struct cnum64 cnum);
 struct cnum64 cnum64_add(struct cnum64 a, struct cnum64 b);
 struct cnum64 cnum64_negate(struct cnum64 a);
+bool cnum64_is_subset(struct cnum64 outer, struct cnum64 inner);
 
 struct cnum32 cnum32_from_cnum64(struct cnum64 cnum);
 struct cnum64 cnum64_cnum32_intersect(struct cnum64 a, struct cnum32 b);
diff --git a/kernel/bpf/cnum_defs.h b/kernel/bpf/cnum_defs.h
index 3ebd8f723dbb..1f232138b6e9 100644
--- a/kernel/bpf/cnum_defs.h
+++ b/kernel/bpf/cnum_defs.h
@@ -220,6 +220,20 @@ bool FN(is_const)(struct cnum_t cnum)
 	return cnum.size == 0;
 }
 
+bool FN(is_subset)(struct cnum_t bigger, struct cnum_t smaller)
+{
+	if (FN(is_empty(smaller)))
+		return true;
+	if (FN(is_empty(bigger)))
+		return false;
+	/* rotate both arcs such that 'bigger' starts at origin, hence does not overflow */
+	smaller.base -= bigger.base;
+	bigger.base = 0;
+	if (FN(urange_overflow)(smaller) && bigger.size < UT_MAX)
+		return false;
+	return smaller.base + smaller.size <= bigger.size;
+}
+
 #undef EMPTY
 #undef cnum_t
 #undef ut
diff --git a/kernel/bpf/states.c b/kernel/bpf/states.c
index a78ae891b743..bd9c22945050 100644
--- a/kernel/bpf/states.c
+++ b/kernel/bpf/states.c
@@ -2,6 +2,7 @@
 /* Copyright (c) 2026 Meta Platforms, Inc. and affiliates. */
 #include <linux/bpf.h>
 #include <linux/bpf_verifier.h>
+#include <linux/cnum.h>
 #include <linux/filter.h>
 
 #define verbose(env, fmt, args...) bpf_verifier_log_write(env, fmt, ##args)
@@ -301,14 +302,8 @@ int bpf_update_branch_counts(struct bpf_verifier_env *env, struct bpf_verifier_s
 static bool range_within(const struct bpf_reg_state *old,
 			 const struct bpf_reg_state *cur)
 {
-	return reg_umin(old) <= reg_umin(cur) &&
-	       reg_umax(old) >= reg_umax(cur) &&
-	       reg_smin(old) <= reg_smin(cur) &&
-	       reg_smax(old) >= reg_smax(cur) &&
-	       reg_u32_min(old) <= reg_u32_min(cur) &&
-	       reg_u32_max(old) >= reg_u32_max(cur) &&
-	       reg_s32_min(old) <= reg_s32_min(cur) &&
-	       reg_s32_max(old) >= reg_s32_max(cur);
+	return cnum64_is_subset(old->r64, cur->r64) &&
+	       cnum32_is_subset(old->r32, cur->r32);
 }
 
 /* If in the old state two registers had the same id, then they need to have

-- 
2.53.0

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH bpf-next 2/2] selftests/bpf: a test for proper cnums compare in is_state_visited()
  2026-04-25 22:48 [PATCH bpf-next 0/2] bpf: range_within() must check cnum ranges instead of min/max pairs Eduard Zingerman
  2026-04-25 22:48 ` [PATCH bpf-next 1/2] " Eduard Zingerman
@ 2026-04-25 22:48 ` Eduard Zingerman
  1 sibling, 0 replies; 5+ messages in thread
From: Eduard Zingerman @ 2026-04-25 22:48 UTC (permalink / raw)
  To: bpf, ast, andrii; +Cc: daniel, martin.lau, kernel-team, yonghong.song, eddyz87

Test case demonstrating a bug in cnum comparison logic fixed by
previous commit. A pruning point is reached with r6 in two states:
1. 32-bit range of [0x7FFFFFF0, U32_MAX] ∪ [0, 0x10]
2. 32-bit range of [0x100, 0x200]

At pruning point the buggy is_state_visited() logic would assume that
would assume range (2) to be a subset of (1) and fail to explore the
path performing division by zero.

Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
---
 .../testing/selftests/bpf/progs/verifier_bounds.c  | 27 ++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/tools/testing/selftests/bpf/progs/verifier_bounds.c b/tools/testing/selftests/bpf/progs/verifier_bounds.c
index 5dd243e653c9..a3e4c0945137 100644
--- a/tools/testing/selftests/bpf/progs/verifier_bounds.c
+++ b/tools/testing/selftests/bpf/progs/verifier_bounds.c
@@ -2267,4 +2267,31 @@ __naked void deduce64_from_32_wrapping_32bit(void)
 	: __clobber_all);
 }
 
+/* Check that range_within() compares cnum ranges, not min/max projections. */
+SEC("socket")
+__failure __msg("div by zero")
+__flag(BPF_F_TEST_STATE_FREQ)
+__naked void range_within_cnum_cross_both_boundaries(void)
+{
+	asm volatile ("							\
+	call %[bpf_get_prandom_u32];					\
+	r1 = 0x80000020;						\
+	if r0 > r1 goto 1f;						\
+	r0 += 0x7FFFFFF0;			/* PATH 1 */		\
+	goto 2f;							\
+1:	call %[bpf_get_prandom_u32];		/* PATH 2 */		\
+	if r0 < 0x100 goto 3f;						\
+	if r0 > 0x200 goto 3f;						\
+2:	/* PATH 1: r0 ∈ [0x7FFFFFF0, U32_MAX] ∪ [0, 0x10] */		\
+	/* PATH 2: r0 ∈ [0x100, 0x200] */				\
+	if r0 != 0x100 goto 3f;	/* True only on PATH 2 */		\
+	r0 /= 0;							\
+3:	exit;								\
+	"
+	:: __imm(bpf_map_lookup_elem),
+	   __imm_addr(map_hash_8b),
+	   __imm(bpf_get_prandom_u32)
+	: __clobber_all);
+}
+
 char _license[] SEC("license") = "GPL";

-- 
2.53.0

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH bpf-next 1/2] bpf: range_within() must check cnum ranges instead of min/max pairs
  2026-04-25 22:48 ` [PATCH bpf-next 1/2] " Eduard Zingerman
@ 2026-04-25 23:20   ` bot+bpf-ci
  2026-04-27 17:56     ` Alexei Starovoitov
  0 siblings, 1 reply; 5+ messages in thread
From: bot+bpf-ci @ 2026-04-25 23:20 UTC (permalink / raw)
  To: eddyz87, bpf, ast, andrii
  Cc: daniel, martin.lau, kernel-team, yonghong.song, eddyz87, ast,
	andrii, daniel, martin.lau, eddyz87, yonghong.song, clm,
	ihor.solodrai

[-- Attachment #1: Type: text/plain, Size: 1932 bytes --]

> bpf: range_within() must check cnum ranges instead of min/max pairs
>
> states.c:range_within() must be updated to properly check if
> cnum-based range in an old state is a superset of a range in the cur
> state. Currently it makes the decision using min/max accessors:
>
>   reg_umin(old) <= reg_umin(cur) <= reg_umax(old)
>
> This is wrong for cnums that cross both UT_MAX/0 and ST_MAX/ST_MIN
> boundaries. Consider cnum32{base=0x7FFFFFF0, size=0x80000020},
> which represents values [0x7FFFFFF0, ..., U32_MAX, 0, ..., 0x10].
> Its projections are u32_min/max=0/U32_MAX, s32_min/max=S32_MIN/MAX.
> A register with range [0x100, 0x200] (which lies entirely in the gap
> of the wrapping range) would pass the min/max check despite having no
> overlap with the actual cnum arc.
>
> This commit replaces min/max comparison with cnum{32,64}_is_subset()
> operation. The operation implementation is verified using cbmc model
> checker in [1].
>
> [1] https://github.com/eddyz87/cnum-verif/
>
> Fixes: bbc631085503 ("bpf: replace min/max fields with struct cnum{32,64}")

The Fixes tag points to commit bbc631085503, but that commit exists in
bpf-next/master and is not in the ancestry of the current branch
(patch-series.local). The current branch is based on a grafted commit
e06021ed87b0 that contains the same cnum-based code with the same bug.

While the Fixes tag is semantically correct (the bug was introduced when
min/max fields were replaced with cnum structures), the referenced commit
is unreachable from this branch. Should the Fixes tag reference the grafted
commit instead, or is this patch intended for a different branch?

> Signed-off-by: Eduard Zingerman <eddyz87@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/24942703368

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH bpf-next 1/2] bpf: range_within() must check cnum ranges instead of min/max pairs
  2026-04-25 23:20   ` bot+bpf-ci
@ 2026-04-27 17:56     ` Alexei Starovoitov
  0 siblings, 0 replies; 5+ messages in thread
From: Alexei Starovoitov @ 2026-04-27 17:56 UTC (permalink / raw)
  To: bot+bpf-ci
  Cc: Eduard, bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
	Martin KaFai Lau, Kernel Team, Yonghong Song, Martin KaFai Lau,
	Chris Mason, Ihor Solodrai

On Sun, Apr 26, 2026 at 12:20 AM <bot+bpf-ci@kernel.org> wrote:
>
> > bpf: range_within() must check cnum ranges instead of min/max pairs
> >
> > states.c:range_within() must be updated to properly check if
> > cnum-based range in an old state is a superset of a range in the cur
> > state. Currently it makes the decision using min/max accessors:
> >
> >   reg_umin(old) <= reg_umin(cur) <= reg_umax(old)
> >
> > This is wrong for cnums that cross both UT_MAX/0 and ST_MAX/ST_MIN
> > boundaries. Consider cnum32{base=0x7FFFFFF0, size=0x80000020},
> > which represents values [0x7FFFFFF0, ..., U32_MAX, 0, ..., 0x10].
> > Its projections are u32_min/max=0/U32_MAX, s32_min/max=S32_MIN/MAX.
> > A register with range [0x100, 0x200] (which lies entirely in the gap
> > of the wrapping range) would pass the min/max check despite having no
> > overlap with the actual cnum arc.
> >
> > This commit replaces min/max comparison with cnum{32,64}_is_subset()
> > operation. The operation implementation is verified using cbmc model
> > checker in [1].
> >
> > [1] https://github.com/eddyz87/cnum-verif/
> >
> > Fixes: bbc631085503 ("bpf: replace min/max fields with struct cnum{32,64}")
>
> The Fixes tag points to commit bbc631085503, but that commit exists in
> bpf-next/master and is not in the ancestry of the current branch
> (patch-series.local). The current branch is based on a grafted commit
> e06021ed87b0 that contains the same cnum-based code with the same bug.
>
> While the Fixes tag is semantically correct (the bug was introduced when
> min/max fields were replaced with cnum structures), the referenced commit
> is unreachable from this branch. Should the Fixes tag reference the grafted
> commit instead, or is this patch intended for a different branch?

AI is being confused.
These patches were applied.

ALL,

please periodically 'git pull' bpf-next to see what was applied.

pw-bot was unreliable in the past and got very spotty recently.
I think it misses every other applied patch.
Maintainers don't have capacity to send manual emails with "Applied. Thanks".

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-04-27 17:56 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-25 22:48 [PATCH bpf-next 0/2] bpf: range_within() must check cnum ranges instead of min/max pairs Eduard Zingerman
2026-04-25 22:48 ` [PATCH bpf-next 1/2] " Eduard Zingerman
2026-04-25 23:20   ` bot+bpf-ci
2026-04-27 17:56     ` Alexei Starovoitov
2026-04-25 22:48 ` [PATCH bpf-next 2/2] selftests/bpf: a test for proper cnums compare in is_state_visited() Eduard Zingerman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox