[PATCH 4.19 0/4] bpf: fix CVE-2021-4159

[PATCH 4.19 0/4] bpf: fix CVE-2021-4159

[Ovidiu Panait]

All verifier selftests pass in qemu for x86-64 with this series applied:
root@intel-x86-64:~# ./test_verifier
...
#664/p mov64 src == dst OK
#665/p mov64 src != dst OK
#666/u calls: ctx read at start of subprog OK
#666/p calls: ctx read at start of subprog OK
Summary: 932 PASSED, 0 SKIPPED, 0 FAILED

Jean-Philippe Brucker (1):
  selftests/bpf: Fix "dubious pointer arithmetic" test

John Fastabend (1):
  bpf: Verifer, adjust_scalar_min_max_vals to always call
    update_reg_bounds()

Maxim Mikityanskiy (1):
  selftests/bpf: add selftest part of "bpf: Fix the off-by-two error in
    range markings"

Stanislav Fomichev (1):
  selftests/bpf: Fix test_align verifier log patterns

 kernel/bpf/verifier.c                       |  1 +
 tools/testing/selftests/bpf/test_align.c    | 41 +++++++++++----------
 tools/testing/selftests/bpf/test_verifier.c | 32 ++++++++--------
 3 files changed, 38 insertions(+), 36 deletions(-)

-- 
2.37.1

[PATCH 4.19 1/4] selftests/bpf: add selftest part of "bpf: Fix the off-by-two error in range markings"

[Ovidiu Panait]

From: Maxim Mikityanskiy <maximmi@nvidia.com>

The 4.19 backport of upstream commit 2fa7d94afc1a ("bpf: Fix the off-by-two
error in range markings") did not include the selftest changes, so currently
there are 8 verifier selftests that are failing:
 # root@intel-x86-64:~# ./test_verifier
 ...
 #495/p XDP pkt read, pkt_end > pkt_data', bad access 1 FAIL
 #498/p XDP pkt read, pkt_data' < pkt_end, bad access 1 FAIL
 #504/p XDP pkt read, pkt_data' >= pkt_end, bad access 1 FAIL
 #513/p XDP pkt read, pkt_end <= pkt_data', bad access 1 FAIL
 #519/p XDP pkt read, pkt_data > pkt_meta', bad access 1 FAIL
 #522/p XDP pkt read, pkt_meta' < pkt_data, bad access 1 FAIL
 #528/p XDP pkt read, pkt_meta' >= pkt_data, bad access 1 FAIL
 #537/p XDP pkt read, pkt_data <= pkt_meta', bad access 1 FAIL
 Summary: 924 PASSED, 0 SKIPPED, 8 FAILED

Cherry-pick the selftest changes to fix these.

Fixes: c315bd962528 ("bpf: Fix the off-by-two error in range markings")
Signed-off-by: Ovidiu Panait <ovidiu.panait@windriver.com>
---
 tools/testing/selftests/bpf/test_verifier.c | 32 ++++++++++-----------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c
index 858e55143233..9a103bd3542c 100644
--- a/tools/testing/selftests/bpf/test_verifier.c
+++ b/tools/testing/selftests/bpf/test_verifier.c
@@ -9108,10 +9108,10 @@ static struct bpf_test tests[] = {
 			BPF_LDX_MEM(BPF_W, BPF_REG_3, BPF_REG_1,
 				    offsetof(struct xdp_md, data_end)),
 			BPF_MOV64_REG(BPF_REG_1, BPF_REG_2),
-			BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, 8),
+			BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, 6),
 			BPF_JMP_REG(BPF_JGT, BPF_REG_3, BPF_REG_1, 1),
 			BPF_JMP_IMM(BPF_JA, 0, 0, 1),
-			BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_1, -8),
+			BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_1, -6),
 			BPF_MOV64_IMM(BPF_REG_0, 0),
 			BPF_EXIT_INSN(),
 		},
@@ -9166,10 +9166,10 @@ static struct bpf_test tests[] = {
 			BPF_LDX_MEM(BPF_W, BPF_REG_3, BPF_REG_1,
 				    offsetof(struct xdp_md, data_end)),
 			BPF_MOV64_REG(BPF_REG_1, BPF_REG_2),
-			BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, 8),
+			BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, 6),
 			BPF_JMP_REG(BPF_JLT, BPF_REG_1, BPF_REG_3, 1),
 			BPF_JMP_IMM(BPF_JA, 0, 0, 1),
-			BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_1, -8),
+			BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_1, -6),
 			BPF_MOV64_IMM(BPF_REG_0, 0),
 			BPF_EXIT_INSN(),
 		},
@@ -9279,9 +9279,9 @@ static struct bpf_test tests[] = {
 			BPF_LDX_MEM(BPF_W, BPF_REG_3, BPF_REG_1,
 				    offsetof(struct xdp_md, data_end)),
 			BPF_MOV64_REG(BPF_REG_1, BPF_REG_2),
-			BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, 8),
+			BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, 6),
 			BPF_JMP_REG(BPF_JGE, BPF_REG_1, BPF_REG_3, 1),
-			BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_1, -8),
+			BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_1, -6),
 			BPF_MOV64_IMM(BPF_REG_0, 0),
 			BPF_EXIT_INSN(),
 		},
@@ -9451,9 +9451,9 @@ static struct bpf_test tests[] = {
 			BPF_LDX_MEM(BPF_W, BPF_REG_3, BPF_REG_1,
 				    offsetof(struct xdp_md, data_end)),
 			BPF_MOV64_REG(BPF_REG_1, BPF_REG_2),
-			BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, 8),
+			BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, 6),
 			BPF_JMP_REG(BPF_JLE, BPF_REG_3, BPF_REG_1, 1),
-			BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_1, -8),
+			BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_1, -6),
 			BPF_MOV64_IMM(BPF_REG_0, 0),
 			BPF_EXIT_INSN(),
 		},
@@ -9564,10 +9564,10 @@ static struct bpf_test tests[] = {
 			BPF_LDX_MEM(BPF_W, BPF_REG_3, BPF_REG_1,
 				    offsetof(struct xdp_md, data)),
 			BPF_MOV64_REG(BPF_REG_1, BPF_REG_2),
-			BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, 8),
+			BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, 6),
 			BPF_JMP_REG(BPF_JGT, BPF_REG_3, BPF_REG_1, 1),
 			BPF_JMP_IMM(BPF_JA, 0, 0, 1),
-			BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_1, -8),
+			BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_1, -6),
 			BPF_MOV64_IMM(BPF_REG_0, 0),
 			BPF_EXIT_INSN(),
 		},
@@ -9622,10 +9622,10 @@ static struct bpf_test tests[] = {
 			BPF_LDX_MEM(BPF_W, BPF_REG_3, BPF_REG_1,
 				    offsetof(struct xdp_md, data)),
 			BPF_MOV64_REG(BPF_REG_1, BPF_REG_2),
-			BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, 8),
+			BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, 6),
 			BPF_JMP_REG(BPF_JLT, BPF_REG_1, BPF_REG_3, 1),
 			BPF_JMP_IMM(BPF_JA, 0, 0, 1),
-			BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_1, -8),
+			BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_1, -6),
 			BPF_MOV64_IMM(BPF_REG_0, 0),
 			BPF_EXIT_INSN(),
 		},
@@ -9735,9 +9735,9 @@ static struct bpf_test tests[] = {
 			BPF_LDX_MEM(BPF_W, BPF_REG_3, BPF_REG_1,
 				    offsetof(struct xdp_md, data)),
 			BPF_MOV64_REG(BPF_REG_1, BPF_REG_2),
-			BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, 8),
+			BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, 6),
 			BPF_JMP_REG(BPF_JGE, BPF_REG_1, BPF_REG_3, 1),
-			BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_1, -8),
+			BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_1, -6),
 			BPF_MOV64_IMM(BPF_REG_0, 0),
 			BPF_EXIT_INSN(),
 		},
@@ -9907,9 +9907,9 @@ static struct bpf_test tests[] = {
 			BPF_LDX_MEM(BPF_W, BPF_REG_3, BPF_REG_1,
 				    offsetof(struct xdp_md, data)),
 			BPF_MOV64_REG(BPF_REG_1, BPF_REG_2),
-			BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, 8),
+			BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, 6),
 			BPF_JMP_REG(BPF_JLE, BPF_REG_3, BPF_REG_1, 1),
-			BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_1, -8),
+			BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_1, -6),
 			BPF_MOV64_IMM(BPF_REG_0, 0),
 			BPF_EXIT_INSN(),
 		},
-- 
2.37.1

[PATCH 4.19 2/4] bpf: Verifer, adjust_scalar_min_max_vals to always call update_reg_bounds()

[Ovidiu Panait]

From: John Fastabend <john.fastabend@gmail.com>

commit 294f2fc6da27620a506e6c050241655459ccd6bd upstream.

Currently, for all op verification we call __red_deduce_bounds() and
__red_bound_offset() but we only call __update_reg_bounds() in bitwise
ops. However, we could benefit from calling __update_reg_bounds() in
BPF_ADD, BPF_SUB, and BPF_MUL cases as well.

For example, a register with state 'R1_w=invP0' when we subtract from
it,

 w1 -= 2

Before coerce we will now have an smin_value=S64_MIN, smax_value=U64_MAX
and unsigned bounds umin_value=0, umax_value=U64_MAX. These will then
be clamped to S32_MIN, U32_MAX values by coerce in the case of alu32 op
as done in above example. However tnum will be a constant because the
ALU op is done on a constant.

Without update_reg_bounds() we have a scenario where tnum is a const
but our unsigned bounds do not reflect this. By calling update_reg_bounds
after coerce to 32bit we further refine the umin_value to U64_MAX in the
alu64 case or U32_MAX in the alu32 case above.

Signed-off-by: John Fastabend <john.fastabend@gmail.com>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Link: https://lore.kernel.org/bpf/158507151689.15666.566796274289413203.stgit@john-Precision-5820-Tower
Signed-off-by: Ovidiu Panait <ovidiu.panait@windriver.com>
---
 kernel/bpf/verifier.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 30ac8ee8294c..694ee0b1fefe 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -3496,6 +3496,7 @@ static int adjust_scalar_min_max_vals(struct bpf_verifier_env *env,
 		coerce_reg_to_size(dst_reg, 4);
 	}
 
+	__update_reg_bounds(dst_reg);
 	__reg_deduce_bounds(dst_reg);
 	__reg_bound_offset(dst_reg);
 	return 0;
-- 
2.37.1

[PATCH 4.19 3/4] selftests/bpf: Fix test_align verifier log patterns

[Ovidiu Panait]

From: Stanislav Fomichev <sdf@google.com>

commit 5366d2269139ba8eb6a906d73a0819947e3e4e0a upstream.

Commit 294f2fc6da27 ("bpf: Verifer, adjust_scalar_min_max_vals to always
call update_reg_bounds()") changed the way verifier logs some of its state,
adjust the test_align accordingly. Where possible, I tried to not copy-paste
the entire log line and resorted to dropping the last closing brace instead.

Fixes: 294f2fc6da27 ("bpf: Verifer, adjust_scalar_min_max_vals to always call update_reg_bounds()")
Signed-off-by: Stanislav Fomichev <sdf@google.com>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Link: https://lore.kernel.org/bpf/20200515194904.229296-1-sdf@google.com
[OP: adjust for 4.19 selftests]
Signed-off-by: Ovidiu Panait <ovidiu.panait@windriver.com>
---
 tools/testing/selftests/bpf/test_align.c | 41 ++++++++++++------------
 1 file changed, 21 insertions(+), 20 deletions(-)

diff --git a/tools/testing/selftests/bpf/test_align.c b/tools/testing/selftests/bpf/test_align.c
index 3c789d03b629..7e057b47b27a 100644
--- a/tools/testing/selftests/bpf/test_align.c
+++ b/tools/testing/selftests/bpf/test_align.c
@@ -359,15 +359,15 @@ static struct bpf_align_test tests[] = {
 			 * is still (4n), fixed offset is not changed.
 			 * Also, we create a new reg->id.
 			 */
-			{29, "R5_w=pkt(id=4,off=18,r=0,umax_value=2040,var_off=(0x0; 0x7fc))"},
+			{29, "R5_w=pkt(id=4,off=18,r=0,umax_value=2040,var_off=(0x0; 0x7fc)"},
 			/* At the time the word size load is performed from R5,
 			 * its total fixed offset is NET_IP_ALIGN + reg->off (18)
 			 * which is 20.  Then the variable offset is (4n), so
 			 * the total offset is 4-byte aligned and meets the
 			 * load's requirements.
 			 */
-			{33, "R4=pkt(id=4,off=22,r=22,umax_value=2040,var_off=(0x0; 0x7fc))"},
-			{33, "R5=pkt(id=4,off=18,r=22,umax_value=2040,var_off=(0x0; 0x7fc))"},
+			{33, "R4=pkt(id=4,off=22,r=22,umax_value=2040,var_off=(0x0; 0x7fc)"},
+			{33, "R5=pkt(id=4,off=18,r=22,umax_value=2040,var_off=(0x0; 0x7fc)"},
 		},
 	},
 	{
@@ -410,15 +410,15 @@ static struct bpf_align_test tests[] = {
 			/* Adding 14 makes R6 be (4n+2) */
 			{9, "R6_w=inv(id=0,umin_value=14,umax_value=1034,var_off=(0x2; 0x7fc))"},
 			/* Packet pointer has (4n+2) offset */
-			{11, "R5_w=pkt(id=1,off=0,r=0,umin_value=14,umax_value=1034,var_off=(0x2; 0x7fc))"},
-			{13, "R4=pkt(id=1,off=4,r=0,umin_value=14,umax_value=1034,var_off=(0x2; 0x7fc))"},
+			{11, "R5_w=pkt(id=1,off=0,r=0,umin_value=14,umax_value=1034,var_off=(0x2; 0x7fc)"},
+			{13, "R4=pkt(id=1,off=4,r=0,umin_value=14,umax_value=1034,var_off=(0x2; 0x7fc)"},
 			/* At the time the word size load is performed from R5,
 			 * its total fixed offset is NET_IP_ALIGN + reg->off (0)
 			 * which is 2.  Then the variable offset is (4n+2), so
 			 * the total offset is 4-byte aligned and meets the
 			 * load's requirements.
 			 */
-			{15, "R5=pkt(id=1,off=0,r=4,umin_value=14,umax_value=1034,var_off=(0x2; 0x7fc))"},
+			{15, "R5=pkt(id=1,off=0,r=4,umin_value=14,umax_value=1034,var_off=(0x2; 0x7fc)"},
 			/* Newly read value in R6 was shifted left by 2, so has
 			 * known alignment of 4.
 			 */
@@ -426,15 +426,15 @@ static struct bpf_align_test tests[] = {
 			/* Added (4n) to packet pointer's (4n+2) var_off, giving
 			 * another (4n+2).
 			 */
-			{19, "R5_w=pkt(id=2,off=0,r=0,umin_value=14,umax_value=2054,var_off=(0x2; 0xffc))"},
-			{21, "R4=pkt(id=2,off=4,r=0,umin_value=14,umax_value=2054,var_off=(0x2; 0xffc))"},
+			{19, "R5_w=pkt(id=2,off=0,r=0,umin_value=14,umax_value=2054,var_off=(0x2; 0xffc)"},
+			{21, "R4=pkt(id=2,off=4,r=0,umin_value=14,umax_value=2054,var_off=(0x2; 0xffc)"},
 			/* At the time the word size load is performed from R5,
 			 * its total fixed offset is NET_IP_ALIGN + reg->off (0)
 			 * which is 2.  Then the variable offset is (4n+2), so
 			 * the total offset is 4-byte aligned and meets the
 			 * load's requirements.
 			 */
-			{23, "R5=pkt(id=2,off=0,r=4,umin_value=14,umax_value=2054,var_off=(0x2; 0xffc))"},
+			{23, "R5=pkt(id=2,off=0,r=4,umin_value=14,umax_value=2054,var_off=(0x2; 0xffc)"},
 		},
 	},
 	{
@@ -469,16 +469,16 @@ static struct bpf_align_test tests[] = {
 		.matches = {
 			{4, "R5_w=pkt_end(id=0,off=0,imm=0)"},
 			/* (ptr - ptr) << 2 == unknown, (4n) */
-			{6, "R5_w=inv(id=0,smax_value=9223372036854775804,umax_value=18446744073709551612,var_off=(0x0; 0xfffffffffffffffc))"},
+			{6, "R5_w=inv(id=0,smax_value=9223372036854775804,umax_value=18446744073709551612,var_off=(0x0; 0xfffffffffffffffc)"},
 			/* (4n) + 14 == (4n+2).  We blow our bounds, because
 			 * the add could overflow.
 			 */
-			{7, "R5=inv(id=0,var_off=(0x2; 0xfffffffffffffffc))"},
+			{7, "R5=inv(id=0,smin_value=-9223372036854775806,smax_value=9223372036854775806,umin_value=2,umax_value=18446744073709551614,var_off=(0x2; 0xfffffffffffffffc)"},
 			/* Checked s>=0 */
-			{9, "R5=inv(id=0,umin_value=2,umax_value=9223372036854775806,var_off=(0x2; 0x7ffffffffffffffc))"},
+			{9, "R5=inv(id=0,umin_value=2,umax_value=9223372034707292158,var_off=(0x2; 0x7fffffff7ffffffc)"},
 			/* packet pointer + nonnegative (4n+2) */
-			{11, "R6_w=pkt(id=1,off=0,r=0,umin_value=2,umax_value=9223372036854775806,var_off=(0x2; 0x7ffffffffffffffc))"},
-			{13, "R4=pkt(id=1,off=4,r=0,umin_value=2,umax_value=9223372036854775806,var_off=(0x2; 0x7ffffffffffffffc))"},
+			{11, "R6_w=pkt(id=1,off=0,r=0,umin_value=2,umax_value=9223372034707292158,var_off=(0x2; 0x7fffffff7ffffffc)"},
+			{13, "R4=pkt(id=1,off=4,r=0,umin_value=2,umax_value=9223372034707292158,var_off=(0x2; 0x7fffffff7ffffffc)"},
 			/* NET_IP_ALIGN + (4n+2) == (4n), alignment is fine.
 			 * We checked the bounds, but it might have been able
 			 * to overflow if the packet pointer started in the
@@ -486,7 +486,7 @@ static struct bpf_align_test tests[] = {
 			 * So we did not get a 'range' on R6, and the access
 			 * attempt will fail.
 			 */
-			{15, "R6=pkt(id=1,off=0,r=0,umin_value=2,umax_value=9223372036854775806,var_off=(0x2; 0x7ffffffffffffffc))"},
+			{15, "R6=pkt(id=1,off=0,r=0,umin_value=2,umax_value=9223372034707292158,var_off=(0x2; 0x7fffffff7ffffffc)"},
 		}
 	},
 	{
@@ -528,7 +528,7 @@ static struct bpf_align_test tests[] = {
 			/* New unknown value in R7 is (4n) */
 			{11, "R7_w=inv(id=0,umax_value=1020,var_off=(0x0; 0x3fc))"},
 			/* Subtracting it from R6 blows our unsigned bounds */
-			{12, "R6=inv(id=0,smin_value=-1006,smax_value=1034,var_off=(0x2; 0xfffffffffffffffc))"},
+			{12, "R6=inv(id=0,smin_value=-1006,smax_value=1034,umin_value=2,umax_value=18446744073709551614,var_off=(0x2; 0xfffffffffffffffc)"},
 			/* Checked s>= 0 */
 			{14, "R6=inv(id=0,umin_value=2,umax_value=1034,var_off=(0x2; 0x7fc))"},
 			/* At the time the word size load is performed from R5,
@@ -537,7 +537,8 @@ static struct bpf_align_test tests[] = {
 			 * the total offset is 4-byte aligned and meets the
 			 * load's requirements.
 			 */
-			{20, "R5=pkt(id=1,off=0,r=4,umin_value=2,umax_value=1034,var_off=(0x2; 0x7fc))"},
+			{20, "R5=pkt(id=1,off=0,r=4,umin_value=2,umax_value=1034,var_off=(0x2; 0x7fc)"},
+
 		},
 	},
 	{
@@ -579,18 +580,18 @@ static struct bpf_align_test tests[] = {
 			/* Adding 14 makes R6 be (4n+2) */
 			{11, "R6_w=inv(id=0,umin_value=14,umax_value=74,var_off=(0x2; 0x7c))"},
 			/* Subtracting from packet pointer overflows ubounds */
-			{13, "R5_w=pkt(id=1,off=0,r=8,umin_value=18446744073709551542,umax_value=18446744073709551602,var_off=(0xffffffffffffff82; 0x7c))"},
+			{13, "R5_w=pkt(id=1,off=0,r=8,umin_value=18446744073709551542,umax_value=18446744073709551602,var_off=(0xffffffffffffff82; 0x7c)"},
 			/* New unknown value in R7 is (4n), >= 76 */
 			{15, "R7_w=inv(id=0,umin_value=76,umax_value=1096,var_off=(0x0; 0x7fc))"},
 			/* Adding it to packet pointer gives nice bounds again */
-			{16, "R5_w=pkt(id=2,off=0,r=0,umin_value=2,umax_value=1082,var_off=(0x2; 0x7fc))"},
+			{16, "R5_w=pkt(id=2,off=0,r=0,umin_value=2,umax_value=1082,var_off=(0x2; 0xfffffffc)"},
 			/* At the time the word size load is performed from R5,
 			 * its total fixed offset is NET_IP_ALIGN + reg->off (0)
 			 * which is 2.  Then the variable offset is (4n+2), so
 			 * the total offset is 4-byte aligned and meets the
 			 * load's requirements.
 			 */
-			{20, "R5=pkt(id=2,off=0,r=4,umin_value=2,umax_value=1082,var_off=(0x2; 0x7fc))"},
+			{20, "R5=pkt(id=2,off=0,r=4,umin_value=2,umax_value=1082,var_off=(0x2; 0xfffffffc)"},
 		},
 	},
 };
-- 
2.37.1

[PATCH 4.19 4/4] selftests/bpf: Fix "dubious pointer arithmetic" test

[Ovidiu Panait]

From: Jean-Philippe Brucker <jean-philippe@linaro.org>

commit 3615bdf6d9b19db12b1589861609b4f1c6a8d303 upstream.

The verifier trace changed following a bugfix. After checking the 64-bit
sign, only the upper bit mask is known, not bit 31. Update the test
accordingly.

Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
Acked-by: John Fastabend <john.fastabend@gmail.com>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
[OP: adjust for 4.19 selftests]
Signed-off-by: Ovidiu Panait <ovidiu.panait@windriver.com>
---
 tools/testing/selftests/bpf/test_align.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/bpf/test_align.c b/tools/testing/selftests/bpf/test_align.c
index 7e057b47b27a..30bca0d82ad3 100644
--- a/tools/testing/selftests/bpf/test_align.c
+++ b/tools/testing/selftests/bpf/test_align.c
@@ -475,10 +475,10 @@ static struct bpf_align_test tests[] = {
 			 */
 			{7, "R5=inv(id=0,smin_value=-9223372036854775806,smax_value=9223372036854775806,umin_value=2,umax_value=18446744073709551614,var_off=(0x2; 0xfffffffffffffffc)"},
 			/* Checked s>=0 */
-			{9, "R5=inv(id=0,umin_value=2,umax_value=9223372034707292158,var_off=(0x2; 0x7fffffff7ffffffc)"},
+			{9, "R5=inv(id=0,umin_value=2,umax_value=9223372036854775806,var_off=(0x2; 0x7ffffffffffffffc)"},
 			/* packet pointer + nonnegative (4n+2) */
-			{11, "R6_w=pkt(id=1,off=0,r=0,umin_value=2,umax_value=9223372034707292158,var_off=(0x2; 0x7fffffff7ffffffc)"},
-			{13, "R4=pkt(id=1,off=4,r=0,umin_value=2,umax_value=9223372034707292158,var_off=(0x2; 0x7fffffff7ffffffc)"},
+			{11, "R6_w=pkt(id=1,off=0,r=0,umin_value=2,umax_value=9223372036854775806,var_off=(0x2; 0x7ffffffffffffffc)"},
+			{13, "R4=pkt(id=1,off=4,r=0,umin_value=2,umax_value=9223372036854775806,var_off=(0x2; 0x7ffffffffffffffc)"},
 			/* NET_IP_ALIGN + (4n+2) == (4n), alignment is fine.
 			 * We checked the bounds, but it might have been able
 			 * to overflow if the packet pointer started in the
@@ -486,7 +486,7 @@ static struct bpf_align_test tests[] = {
 			 * So we did not get a 'range' on R6, and the access
 			 * attempt will fail.
 			 */
-			{15, "R6=pkt(id=1,off=0,r=0,umin_value=2,umax_value=9223372034707292158,var_off=(0x2; 0x7fffffff7ffffffc)"},
+			{15, "R6=pkt(id=1,off=0,r=0,umin_value=2,umax_value=9223372036854775806,var_off=(0x2; 0x7ffffffffffffffc)"},
 		}
 	},
 	{
-- 
2.37.1

Re: [PATCH 4.19 1/4] selftests/bpf: add selftest part of "bpf: Fix the off-by-two error in range markings"

[Greg KH]

On Tue, Aug 09, 2022 at 10:39:44AM +0300, Ovidiu Panait wrote:
> From: Maxim Mikityanskiy <maximmi@nvidia.com>
> 
> The 4.19 backport of upstream commit 2fa7d94afc1a ("bpf: Fix the off-by-two
> error in range markings") did not include the selftest changes, so currently
> there are 8 verifier selftests that are failing:
>  # root@intel-x86-64:~# ./test_verifier
>  ...
>  #495/p XDP pkt read, pkt_end > pkt_data', bad access 1 FAIL
>  #498/p XDP pkt read, pkt_data' < pkt_end, bad access 1 FAIL
>  #504/p XDP pkt read, pkt_data' >= pkt_end, bad access 1 FAIL
>  #513/p XDP pkt read, pkt_end <= pkt_data', bad access 1 FAIL
>  #519/p XDP pkt read, pkt_data > pkt_meta', bad access 1 FAIL
>  #522/p XDP pkt read, pkt_meta' < pkt_data, bad access 1 FAIL
>  #528/p XDP pkt read, pkt_meta' >= pkt_data, bad access 1 FAIL
>  #537/p XDP pkt read, pkt_data <= pkt_meta', bad access 1 FAIL
>  Summary: 924 PASSED, 0 SKIPPED, 8 FAILED
> 
> Cherry-pick the selftest changes to fix these.

What specific "selftest changes" are you cherry-picking here?  I can't
take this commit without that reference.

thanks,

greg k-h

Re: [PATCH 4.19 0/4] bpf: fix CVE-2021-4159

[Greg KH]

On Tue, Aug 09, 2022 at 10:39:43AM +0300, Ovidiu Panait wrote:
> All verifier selftests pass in qemu for x86-64 with this series applied:
> root@intel-x86-64:~# ./test_verifier
> ...
> #664/p mov64 src == dst OK
> #665/p mov64 src != dst OK
> #666/u calls: ctx read at start of subprog OK
> #666/p calls: ctx read at start of subprog OK
> Summary: 932 PASSED, 0 SKIPPED, 0 FAILED
> 
> Jean-Philippe Brucker (1):
>   selftests/bpf: Fix "dubious pointer arithmetic" test
> 
> John Fastabend (1):
>   bpf: Verifer, adjust_scalar_min_max_vals to always call
>     update_reg_bounds()
> 
> Maxim Mikityanskiy (1):
>   selftests/bpf: add selftest part of "bpf: Fix the off-by-two error in
>     range markings"
> 
> Stanislav Fomichev (1):
>   selftests/bpf: Fix test_align verifier log patterns
> 
>  kernel/bpf/verifier.c                       |  1 +
>  tools/testing/selftests/bpf/test_align.c    | 41 +++++++++++----------
>  tools/testing/selftests/bpf/test_verifier.c | 32 ++++++++--------
>  3 files changed, 38 insertions(+), 36 deletions(-)
> 
> -- 
> 2.37.1
> 

Patches 2-4 now queued up, thanks.

greg k-h

Re: [PATCH 4.19 1/4] selftests/bpf: add selftest part of "bpf: Fix the off-by-two error in range markings"

[Ovidiu Panait]

Hi Greg,

On 8/13/22 16:10, Greg KH wrote:
> [Please note: This e-mail is from an EXTERNAL e-mail address]
>
> On Tue, Aug 09, 2022 at 10:39:44AM +0300, Ovidiu Panait wrote:
>> From: Maxim Mikityanskiy <maximmi@nvidia.com>
>>
>> The 4.19 backport of upstream commit 2fa7d94afc1a ("bpf: Fix the off-by-two
>> error in range markings") did not include the selftest changes, so currently
>> there are 8 verifier selftests that are failing:
>>   # root@intel-x86-64:~# ./test_verifier
>>   ...
>>   #495/p XDP pkt read, pkt_end > pkt_data', bad access 1 FAIL
>>   #498/p XDP pkt read, pkt_data' < pkt_end, bad access 1 FAIL
>>   #504/p XDP pkt read, pkt_data' >= pkt_end, bad access 1 FAIL
>>   #513/p XDP pkt read, pkt_end <= pkt_data', bad access 1 FAIL
>>   #519/p XDP pkt read, pkt_data > pkt_meta', bad access 1 FAIL
>>   #522/p XDP pkt read, pkt_meta' < pkt_data, bad access 1 FAIL
>>   #528/p XDP pkt read, pkt_meta' >= pkt_data, bad access 1 FAIL
>>   #537/p XDP pkt read, pkt_data <= pkt_meta', bad access 1 FAIL
>>   Summary: 924 PASSED, 0 SKIPPED, 8 FAILED
>>
>> Cherry-pick the selftest changes to fix these.
> What specific "selftest changes" are you cherry-picking here?  I can't
> take this commit without that reference.

This patch includes the selftest part of upstream commit:

https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=2fa7d94afc1a


The 4.19 backport of the above commit did not include the selftest updates:

https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=c315bd962528


Ovidiu

> thanks,
>
> greg k-h

Re: [PATCH 4.19 1/4] selftests/bpf: add selftest part of "bpf: Fix the off-by-two error in range markings"

[Greg KH]

On Sun, Aug 14, 2022 at 04:58:56PM +0300, Ovidiu Panait wrote:
> Hi Greg,
> 
> On 8/13/22 16:10, Greg KH wrote:
> > [Please note: This e-mail is from an EXTERNAL e-mail address]
> > 
> > On Tue, Aug 09, 2022 at 10:39:44AM +0300, Ovidiu Panait wrote:
> > > From: Maxim Mikityanskiy <maximmi@nvidia.com>
> > > 
> > > The 4.19 backport of upstream commit 2fa7d94afc1a ("bpf: Fix the off-by-two
> > > error in range markings") did not include the selftest changes, so currently
> > > there are 8 verifier selftests that are failing:
> > >   # root@intel-x86-64:~# ./test_verifier
> > >   ...
> > >   #495/p XDP pkt read, pkt_end > pkt_data', bad access 1 FAIL
> > >   #498/p XDP pkt read, pkt_data' < pkt_end, bad access 1 FAIL
> > >   #504/p XDP pkt read, pkt_data' >= pkt_end, bad access 1 FAIL
> > >   #513/p XDP pkt read, pkt_end <= pkt_data', bad access 1 FAIL
> > >   #519/p XDP pkt read, pkt_data > pkt_meta', bad access 1 FAIL
> > >   #522/p XDP pkt read, pkt_meta' < pkt_data, bad access 1 FAIL
> > >   #528/p XDP pkt read, pkt_meta' >= pkt_data, bad access 1 FAIL
> > >   #537/p XDP pkt read, pkt_data <= pkt_meta', bad access 1 FAIL
> > >   Summary: 924 PASSED, 0 SKIPPED, 8 FAILED
> > > 
> > > Cherry-pick the selftest changes to fix these.
> > What specific "selftest changes" are you cherry-picking here?  I can't
> > take this commit without that reference.
> 
> This patch includes the selftest part of upstream commit:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=2fa7d94afc1a
> 
> 
> The 4.19 backport of the above commit did not include the selftest updates:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=c315bd962528

I still do not understand, what commit is this coming from that matches
it up with what is in Linus's tree?

confused,

greg k-h

Re: [PATCH 4.19 1/4] selftests/bpf: add selftest part of "bpf: Fix the off-by-two error in range markings"

[Ovidiu Panait]

On 8/14/22 17:44, Greg KH wrote:
> [Please note: This e-mail is from an EXTERNAL e-mail address]
>
> On Sun, Aug 14, 2022 at 04:58:56PM +0300, Ovidiu Panait wrote:
>> Hi Greg,
>>
>> On 8/13/22 16:10, Greg KH wrote:
>>> [Please note: This e-mail is from an EXTERNAL e-mail address]
>>>
>>> On Tue, Aug 09, 2022 at 10:39:44AM +0300, Ovidiu Panait wrote:
>>>> From: Maxim Mikityanskiy <maximmi@nvidia.com>
>>>>
>>>> The 4.19 backport of upstream commit 2fa7d94afc1a ("bpf: Fix the off-by-two
>>>> error in range markings") did not include the selftest changes, so currently
>>>> there are 8 verifier selftests that are failing:
>>>>    # root@intel-x86-64:~# ./test_verifier
>>>>    ...
>>>>    #495/p XDP pkt read, pkt_end > pkt_data', bad access 1 FAIL
>>>>    #498/p XDP pkt read, pkt_data' < pkt_end, bad access 1 FAIL
>>>>    #504/p XDP pkt read, pkt_data' >= pkt_end, bad access 1 FAIL
>>>>    #513/p XDP pkt read, pkt_end <= pkt_data', bad access 1 FAIL
>>>>    #519/p XDP pkt read, pkt_data > pkt_meta', bad access 1 FAIL
>>>>    #522/p XDP pkt read, pkt_meta' < pkt_data, bad access 1 FAIL
>>>>    #528/p XDP pkt read, pkt_meta' >= pkt_data, bad access 1 FAIL
>>>>    #537/p XDP pkt read, pkt_data <= pkt_meta', bad access 1 FAIL
>>>>    Summary: 924 PASSED, 0 SKIPPED, 8 FAILED
>>>>
>>>> Cherry-pick the selftest changes to fix these.
>>> What specific "selftest changes" are you cherry-picking here?  I can't
>>> take this commit without that reference.
>> This patch includes the selftest part of upstream commit:
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=2fa7d94afc1a
>>
>>
>> The 4.19 backport of the above commit did not include the selftest updates:
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=c315bd962528
> I still do not understand, what commit is this coming from that matches
> it up with what is in Linus's tree?
The changes come from commit 2fa7d94afc1a ("bpf: Fix the off-by-two 
error in range markings") in Linus's tree.

Ovidiu

>
> confused,
>
> greg k-h

Re: [PATCH 4.19 1/4] selftests/bpf: add selftest part of "bpf: Fix the off-by-two error in range markings"

[Greg KH]

On Sun, Aug 14, 2022 at 06:08:26PM +0300, Ovidiu Panait wrote:
> 
> On 8/14/22 17:44, Greg KH wrote:
> > [Please note: This e-mail is from an EXTERNAL e-mail address]
> > 
> > On Sun, Aug 14, 2022 at 04:58:56PM +0300, Ovidiu Panait wrote:
> > > Hi Greg,
> > > 
> > > On 8/13/22 16:10, Greg KH wrote:
> > > > [Please note: This e-mail is from an EXTERNAL e-mail address]
> > > > 
> > > > On Tue, Aug 09, 2022 at 10:39:44AM +0300, Ovidiu Panait wrote:
> > > > > From: Maxim Mikityanskiy <maximmi@nvidia.com>
> > > > > 
> > > > > The 4.19 backport of upstream commit 2fa7d94afc1a ("bpf: Fix the off-by-two
> > > > > error in range markings") did not include the selftest changes, so currently
> > > > > there are 8 verifier selftests that are failing:
> > > > >    # root@intel-x86-64:~# ./test_verifier
> > > > >    ...
> > > > >    #495/p XDP pkt read, pkt_end > pkt_data', bad access 1 FAIL
> > > > >    #498/p XDP pkt read, pkt_data' < pkt_end, bad access 1 FAIL
> > > > >    #504/p XDP pkt read, pkt_data' >= pkt_end, bad access 1 FAIL
> > > > >    #513/p XDP pkt read, pkt_end <= pkt_data', bad access 1 FAIL
> > > > >    #519/p XDP pkt read, pkt_data > pkt_meta', bad access 1 FAIL
> > > > >    #522/p XDP pkt read, pkt_meta' < pkt_data, bad access 1 FAIL
> > > > >    #528/p XDP pkt read, pkt_meta' >= pkt_data, bad access 1 FAIL
> > > > >    #537/p XDP pkt read, pkt_data <= pkt_meta', bad access 1 FAIL
> > > > >    Summary: 924 PASSED, 0 SKIPPED, 8 FAILED
> > > > > 
> > > > > Cherry-pick the selftest changes to fix these.
> > > > What specific "selftest changes" are you cherry-picking here?  I can't
> > > > take this commit without that reference.
> > > This patch includes the selftest part of upstream commit:
> > > 
> > > https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=2fa7d94afc1a
> > > 
> > > 
> > > The 4.19 backport of the above commit did not include the selftest updates:
> > > 
> > > https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=c315bd962528
> > I still do not understand, what commit is this coming from that matches
> > it up with what is in Linus's tree?
> The changes come from commit 2fa7d94afc1a ("bpf: Fix the off-by-two error in
> range markings") in Linus's tree.

Then can you please reword the text here to make it obvious what is
happening and resubmit?

thanks,

gre gk-h