bpf: Use tnums for JEQ/JNE is_branch_taken logic

[ Upstream commit f41345f47fb267a9c95ca710c33448f8d0d81d83 ]

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.

Signed-off-by: Paul Chaignon <paul.chaignon@gmail.com>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Acked-by: Eduard Zingerman <eddyz87@gmail.com>
Acked-by: Shung-Hsi Yu <shung-hsi.yu@suse.com>
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
Link: https://lore.kernel.org/bpf/be3ee70b6e489c49881cb1646114b1d861b5c334.1755694147.git.paul.chaignon@gmail.com
Signed-off-by: Sasha Levin <sashal@kernel.org>

diff --git a/include/linux/tnum.h b/include/linux/tnum.h
index 3c13240077b87a70eb73559fcb8f17dd41a6da3f..191baf3a8d832e0d0afb9af8935ae1ec0f56c627 100644 (file)
--- a/include/linux/tnum.h
+++ b/include/linux/tnum.h
@@ -49,6 +49,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 9dbc31b25e3d08f4424fc8cf298944018acc85a1..8e6d0ac7137311747972b8414c82c17bd743bc16 100644 (file)
--- a/kernel/bpf/tnum.c
+++ b/kernel/bpf/tnum.c
@@ -138,6 +138,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 709151d33e5e467384e067e20b4675e330977cb7..218c238d6139895cda6f9cd1e95b8051c72ec2df 100644 (file)
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -14772,6 +14772,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;
@@ -14796,6 +14798,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;