DPDK-dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 12/25] bpf/validate: fix BPF_DIV and BPF_MOD signed part
From: Marat Khalili @ 2026-06-12 10:47 UTC (permalink / raw)
  To: Konstantin Ananyev; +Cc: dev, stable, Claudia Cauli
In-Reply-To: <20260612104743.6465-1-marat.khalili@huawei.com>

Function `eval_divmod` for _unsigned_ division or modulo operation
calculated signed ranges using _signed_ division, which is
mathematically incorrect: unlike some other mathematical operations,
signed and unsigned divisions in the CPU register cyclic ring math are
not equivalent.

E.g. consider the following program with the current validation code:

     Tested program:
         0:  mov r0, #0x0
         1:  lddw r2, #0xaaaaaaaaaaaaaaaa
         3:  mov r3, #0x2
         4:  div r2, r3  ; tested instruction
         5:  mov r0, #0x1
         6:  exit
     Pre-state:
        r2:  -6148914691236517206
        r3:  2
     Post-state:
        r2:  -3074457345618258603 INTERSECT 0x5555555555555555 (!)

After the tested instruction validator considers r2 to equal
0x5555555555555555 if viewed as unsigned (correct, this is
0xaaaaaaaaaaaaaaaaull / 2), but equal -3074457345618258603 or
0xd555555555555555 if viewed as signed, although it cannot be both true.

Additionally, when validating division or modulo of INT64_MIN by -1
overflow happened in the validator possibly triggering an exception.

The following error is shown without sanitizer:

    1/1 DPDK:fast-tests / bpf_autotest FAIL            0.37s
    killed by signal 8 SIGFPE

With sanitizer the following diagnostic is generated:

    lib/bpf/bpf_validate.c:1086:14: runtime error: division of
    -9223372036854775808 by -1 cannot be represented in type 'long int'
        #0 0x0000027484bb in eval_divmod lib/bpf/bpf_validate.c:1086
        #1 0x00000274bcf3 in eval_alu lib/bpf/bpf_validate.c:1280
        #2 0x00000275cb3e in evaluate lib/bpf/bpf_validate.c:3192
        ...

    SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior
    lib/bpf/bpf_validate.c:1086:14

Change logic to copy results from unsigned division into signed. Add
both validation and execution tests for the case that triggered an
exception. Add validation tests for non-constant division to make sure
it is still valid (ranges of the non-constant division or modulo are not
really minimal, this can be addressed in the future).

Fixes: 8021917293d0 ("bpf: add extra validation for input BPF program")
Cc: stable@dpdk.org

Reported-by: Claudia Cauli <claudiacauli@gmail.com>
Signed-off-by: Marat Khalili <marat.khalili@huawei.com>
Acked-by: Konstantin Ananyev <konstantin.ananyev@huawei.com>
---
Depends-on: series-38434 ("bpf: introduce extensible load API")
 app/test/test_bpf.c          |  99 +++++++++++++++++++++++++
 app/test/test_bpf_validate.c | 135 +++++++++++++++++++++++++++++++++++
 lib/bpf/bpf_validate.c       |  38 +++-------
 3 files changed, 244 insertions(+), 28 deletions(-)

diff --git a/app/test/test_bpf.c b/app/test/test_bpf.c
index 6422bae6fe5f..82ff12365307 100644
--- a/app/test/test_bpf.c
+++ b/app/test/test_bpf.c
@@ -393,6 +393,13 @@ cmp_res(const char *func, uint64_t exp_rc, uint64_t ret_rc,
 	return ret;
 }
 
+/* Empty prepare function */
+static void
+dummy_prepare(void *arg)
+{
+	RTE_SET_USED(arg);
+}
+
 /* store immediate test-cases */
 static const struct ebpf_insn test_store1_prog[] = {
 	{
@@ -3157,6 +3164,70 @@ static const struct ebpf_insn test_ld_mbuf3_prog[] = {
 	},
 };
 
+/* divide INT64_MIN by -1 */
+static const struct ebpf_insn test_int64min_udiv_uint64max_prog[] = {
+	/* Load INT64_MIN into r0 */
+	{
+		.code = (BPF_LD | BPF_IMM | EBPF_DW),
+		.dst_reg = EBPF_REG_0,
+		.imm = (int32_t)INT64_MIN,
+	},
+	{
+		.imm = (int32_t)(INT64_MIN >> 32),
+	},
+	/* Divide r0 by immediate -1 */
+	{
+		.code = (EBPF_ALU64 | BPF_DIV | BPF_K),
+		.dst_reg = EBPF_REG_0,
+		.imm = -1,
+	},
+	/* Exit for correctness otherwise */
+	{
+		.code = (BPF_JMP | EBPF_EXIT),
+	},
+};
+
+static int
+test_int64min_udiv_uint64max_check(uint64_t rc, const void *arg)
+{
+	RTE_SET_USED(arg);
+	/* 0x8000000000000000ull / 0xFFFFFFFFFFFFFFFFull == 0 */
+	TEST_ASSERT_EQUAL(rc, 0, "expected 0, found %#" PRIx64, rc);
+	return TEST_SUCCESS;
+}
+
+/* modulo INT64_MIN by -1 */
+static const struct ebpf_insn test_int64min_umod_uint64max_prog[] = {
+	/* Load INT64_MIN into r0 */
+	{
+		.code = (BPF_LD | BPF_IMM | EBPF_DW),
+		.dst_reg = EBPF_REG_0,
+		.imm = (int32_t)INT64_MIN,
+	},
+	{
+		.imm = (int32_t)(INT64_MIN >> 32),
+	},
+	/* Modulo r0 by immediate -1 */
+	{
+		.code = (EBPF_ALU64 | BPF_MOD | BPF_K),
+		.dst_reg = EBPF_REG_0,
+		.imm = -1,
+	},
+	/* Exit for correctness otherwise */
+	{
+		.code = (BPF_JMP | EBPF_EXIT),
+	},
+};
+
+static int
+test_int64min_umod_uint64max_check(uint64_t rc, const void *arg)
+{
+	RTE_SET_USED(arg);
+	/* 0x8000000000000000ull % 0xFFFFFFFFFFFFFFFFull == 0x8000000000000000ull  */
+	TEST_ASSERT_EQUAL(rc, (uint64_t)INT64_MIN, "expected INT64_MIN, found %#" PRIx64, rc);
+	return TEST_SUCCESS;
+}
+
 /* all bpf test cases */
 static const struct bpf_test tests[] = {
 	{
@@ -3465,6 +3536,34 @@ static const struct bpf_test tests[] = {
 		/* mbuf as input argument is not supported on 32 bit platform */
 		.allow_fail = (sizeof(uint64_t) != sizeof(uintptr_t)),
 	},
+	{
+		.name = "test_int64min_udiv_uint64max",
+		.arg_sz = sizeof(struct dummy_vect8),
+		.prm = {
+			.ins = test_int64min_udiv_uint64max_prog,
+			.nb_ins = RTE_DIM(test_int64min_udiv_uint64max_prog),
+			.prog_arg = {
+				.type = RTE_BPF_ARG_PTR,
+				.size = sizeof(struct dummy_vect8),
+			},
+		},
+		.prepare = dummy_prepare,
+		.check_result = test_int64min_udiv_uint64max_check,
+	},
+	{
+		.name = "test_int64min_umod_uint64max",
+		.arg_sz = 1,
+		.prm = {
+			.ins = test_int64min_umod_uint64max_prog,
+			.nb_ins = RTE_DIM(test_int64min_umod_uint64max_prog),
+			.prog_arg = {
+				.type = RTE_BPF_ARG_PTR,
+				.size = 1,
+			},
+		},
+		.prepare = dummy_prepare,
+		.check_result = test_int64min_umod_uint64max_check,
+	},
 };
 
 static int
diff --git a/app/test/test_bpf_validate.c b/app/test/test_bpf_validate.c
index c752d8635756..31a235a55af6 100644
--- a/app/test/test_bpf_validate.c
+++ b/app/test/test_bpf_validate.c
@@ -1154,6 +1154,141 @@ test_alu64_add_x_scalar_scalar(void)
 REGISTER_FAST_TEST(bpf_validate_alu64_add_x_scalar_scalar_autotest, NOHUGE_OK, ASAN_OK,
 	test_alu64_add_x_scalar_scalar);
 
+/* 64-bit division and modulo of UINT64_MAX*2/3. */
+static int
+test_alu64_div_mod_big_constant(void)
+{
+	const uint64_t dividend = UINT64_MAX / 3 * 2;
+	static const uint64_t divisors[] = {
+		1,
+		2,
+		3,
+		UINT64_MAX / 3,
+		INT64_MAX,
+		INT64_MIN,
+		UINT64_MAX / 3 * 2,
+		UINT64_MAX / 4 * 3,
+		UINT64_MAX,
+	};
+	for (int index = 0; index != RTE_DIM(divisors); ++index) {
+		const uint64_t divisor = divisors[index];
+
+		TEST_ASSERT_SUCCESS(verify_instruction((struct verify_instruction_param){
+			.tested_instruction = {
+				.code = (EBPF_ALU64 | BPF_DIV | BPF_X),
+			},
+			.pre.dst = make_singleton_domain(dividend),
+			.pre.src = make_singleton_domain(divisor),
+			.post.dst = make_singleton_domain(dividend / divisor),
+		}), "(EBPF_ALU64 | BPF_DIV | BPF_X) check, index=%d", index);
+
+		TEST_ASSERT_SUCCESS(verify_instruction((struct verify_instruction_param){
+			.tested_instruction = {
+				.code = (EBPF_ALU64 | BPF_MOD | BPF_X),
+			},
+			.pre.dst = make_singleton_domain(dividend),
+			.pre.src = make_singleton_domain(divisor),
+			.post.dst = make_singleton_domain(dividend % divisor),
+		}), "(EBPF_ALU64 | BPF_MOD | BPF_X) check, index=%d", index);
+	}
+
+	return TEST_SUCCESS;
+}
+
+REGISTER_FAST_TEST(bpf_validate_alu64_div_mod_big_constant_autotest, NOHUGE_OK, ASAN_OK,
+	test_alu64_div_mod_big_constant);
+
+/* 64-bit division and modulo of UINT64_MAX/3..UINT64_MAX*2/3 by a constant. */
+static int
+test_alu64_div_mod_big_range(void)
+{
+	const uint64_t dividend_first = UINT64_MAX / 3;
+	const uint64_t dividend_last = UINT64_MAX / 3 * 2;
+	static const uint64_t divisors[] = {
+		1,
+		2,
+		3,
+		UINT64_MAX / 3,
+		INT64_MAX,
+		INT64_MIN,
+		UINT64_MAX / 3 * 2,
+		UINT64_MAX / 4 * 3,
+		UINT64_MAX,
+	};
+	for (int index = 0; index != RTE_DIM(divisors); ++index) {
+		const uint64_t divisor = divisors[index];
+
+		TEST_ASSERT_SUCCESS(verify_instruction((struct verify_instruction_param){
+			.tested_instruction = {
+				.code = (EBPF_ALU64 | BPF_DIV | BPF_X),
+			},
+			.pre.dst = make_unsigned_domain(dividend_first, dividend_last),
+			.pre.src = make_singleton_domain(divisor),
+			.post.dst = make_unsigned_domain(0, dividend_last),
+		}), "(EBPF_ALU64 | BPF_DIV | BPF_X) check, index=%d", index);
+
+		TEST_ASSERT_SUCCESS(verify_instruction((struct verify_instruction_param){
+			.tested_instruction = {
+				.code = (EBPF_ALU64 | BPF_MOD | BPF_X),
+			},
+			.pre.dst = make_unsigned_domain(dividend_first, dividend_last),
+			.pre.src = make_singleton_domain(divisor),
+			.post.dst = make_unsigned_domain(0, RTE_MIN(dividend_last, divisor - 1)),
+		}), "(EBPF_ALU64 | BPF_MOD | BPF_X) check, index=%d", index);
+	}
+
+	return TEST_SUCCESS;
+}
+
+REGISTER_FAST_TEST(bpf_validate_alu64_div_mod_big_range_autotest, NOHUGE_OK, ASAN_OK,
+	test_alu64_div_mod_big_range);
+
+/* 64-bit division and modulo of INT64_MIN by -1. */
+static int
+test_alu64_div_mod_overflow(void)
+{
+	TEST_ASSERT_SUCCESS(verify_instruction((struct verify_instruction_param){
+		.tested_instruction = {
+			.code = (EBPF_ALU64 | BPF_DIV | BPF_K),
+			.imm = -1,
+		},
+		.pre.dst = make_singleton_domain(INT64_MIN),
+		.post.dst = make_singleton_domain(0),
+	}), "(EBPF_ALU64 | BPF_DIV | BPF_K) check");
+
+	TEST_ASSERT_SUCCESS(verify_instruction((struct verify_instruction_param){
+		.tested_instruction = {
+			.code = (EBPF_ALU64 | BPF_DIV | BPF_X),
+		},
+		.pre.dst = make_singleton_domain(INT64_MIN),
+		.pre.src = make_singleton_domain(-1),
+		.post.dst = make_singleton_domain(0),
+	}), "(EBPF_ALU64 | BPF_DIV | BPF_X) check");
+
+	TEST_ASSERT_SUCCESS(verify_instruction((struct verify_instruction_param){
+		.tested_instruction = {
+			.code = (EBPF_ALU64 | BPF_MOD | BPF_K),
+			.imm = -1,
+		},
+		.pre.dst = make_singleton_domain(INT64_MIN),
+		.post.dst = make_singleton_domain(INT64_MIN),
+	}), "(EBPF_ALU64 | BPF_MOD | BPF_K) check");
+
+	TEST_ASSERT_SUCCESS(verify_instruction((struct verify_instruction_param){
+		.tested_instruction = {
+			.code = (EBPF_ALU64 | BPF_MOD | BPF_X),
+		},
+		.pre.dst = make_singleton_domain(INT64_MIN),
+		.pre.src = make_singleton_domain(-1),
+		.post.dst = make_singleton_domain(INT64_MIN),
+	}), "(EBPF_ALU64 | BPF_MOD | BPF_X) check");
+
+	return TEST_SUCCESS;
+}
+
+REGISTER_FAST_TEST(bpf_validate_alu64_div_mod_overflow_autotest, NOHUGE_OK, ASAN_OK,
+	test_alu64_div_mod_overflow);
+
 /* 64-bit negation when interval first element is INT64_MIN. */
 static int
 test_alu64_neg_int64min_first(void)
diff --git a/lib/bpf/bpf_validate.c b/lib/bpf/bpf_validate.c
index 38e9b033c6d9..14a186b7cbf7 100644
--- a/lib/bpf/bpf_validate.c
+++ b/lib/bpf/bpf_validate.c
@@ -932,8 +932,7 @@ eval_mul(struct bpf_reg_val *rd, const struct bpf_reg_val *rs, size_t opsz,
 }
 
 static const char *
-eval_divmod(uint32_t op, struct bpf_reg_val *rd, struct bpf_reg_val *rs,
-	size_t opsz, uint64_t msk)
+eval_divmod(uint32_t op, struct bpf_reg_val *rd, struct bpf_reg_val *rs, uint64_t msk)
 {
 	/* both operands are constants */
 	if (rd->u.min == rd->u.max && rs->u.min == rs->u.max) {
@@ -954,34 +953,17 @@ eval_divmod(uint32_t op, struct bpf_reg_val *rd, struct bpf_reg_val *rs,
 		rd->u.min = 0;
 	}
 
-	/* if we have 32-bit values - extend them to 64-bit */
-	if (opsz == sizeof(uint32_t) * CHAR_BIT) {
-		rd->s.min = (int32_t)rd->s.min;
-		rd->s.max = (int32_t)rd->s.max;
-		rs->s.min = (int32_t)rs->s.min;
-		rs->s.max = (int32_t)rs->s.max;
-	}
-
-	/* both operands are constants */
-	if (rd->s.min == rd->s.max && rs->s.min == rs->s.max) {
-		if (rs->s.max == 0)
-			return "division by 0";
-		if (op == BPF_DIV) {
-			rd->s.min /= rs->s.min;
-			rd->s.max /= rs->s.max;
-		} else {
-			rd->s.min %= rs->s.min;
-			rd->s.max %= rs->s.max;
-		}
-	} else if (op == BPF_MOD) {
-		rd->s.min = RTE_MAX(rd->s.max, 0);
-		rd->s.min = RTE_MIN(rd->s.min, 0);
+	if (rd->u.min >= (uint64_t)INT64_MIN || rd->u.max <= (uint64_t)INT64_MAX) {
+		/*
+		 * All values have the same sign bit, which means range
+		 * contiguous as unsigned is also contiguous as signed,
+		 * so we can just reuse it without any changes.
+		 */
+		rd->s.min = rd->u.min;
+		rd->s.max = rd->u.max;
 	} else
 		eval_smax_bound(rd, msk);
 
-	rd->s.max &= msk;
-	rd->s.min &= msk;
-
 	return NULL;
 }
 
@@ -1165,7 +1147,7 @@ eval_alu(struct bpf_verifier *bvf, const struct ebpf_insn *ins)
 	else if (op == BPF_MUL)
 		eval_mul(rd, &rs, opsz, msk);
 	else if (op == BPF_DIV || op == BPF_MOD)
-		err = eval_divmod(op, rd, &rs, opsz, msk);
+		err = eval_divmod(op, rd, &rs, msk);
 	else if (op == BPF_NEG)
 		eval_neg(rd, opsz, msk);
 	else if (op == EBPF_MOV)
-- 
2.43.0


^ permalink raw reply related

* [PATCH v3 07/25] bpf/validate: fix BPF_LDX | EBPF_DW signed range
From: Marat Khalili @ 2026-06-12 10:47 UTC (permalink / raw)
  To: Konstantin Ananyev; +Cc: dev, stable
In-Reply-To: <20260612104743.6465-1-marat.khalili@huawei.com>

Function `eval_max_load` copied signed range from unsigned regardless of
the mask (operation width) producing on 64-bit load nonsensical signed
range 0..-1 that breaks invariant min <= max relied upon in multiple
places (e.g. signed overflow detection in `eval_mul` only checks `s.min`
to make sure the range is non-negative and so on).

E.g. consider the following program with the current validation code:

    Tested program:
        0:  mov r0, #0x0
        1:  mov r3, #0x0
        2:  add r3, r1
        3:  ldxdw r2, [r3 + 16]  ; tested instruction
        4:  mov r0, #0x1
        5:  exit
    Pre-state:
       r2:  %undefined
       r3:  %buffer<24> + 0
    Post-state:
       r2:  0..-1 INTERSECT 0..UINT64_MAX (!)
       r3:  %buffer<24> + 0

Part before INTERSECT represents signed range, part after INTERSECT
represents unsigned range. Unsigned range is correctly set to full range
0..UINT64_MAX, but signed range copied from it becomes 0..-1.

Fix loading logic to only copy unsigned to signed for non-full mask.

The test will be added in subsequent commits since it depends on other
fixes.

Fixes: 8021917293d0 ("bpf: add extra validation for input BPF program")
Cc: stable@dpdk.org

Signed-off-by: Marat Khalili <marat.khalili@huawei.com>
Acked-by: Konstantin Ananyev <konstantin.ananyev@huawei.com>
---
Depends-on: series-38434 ("bpf: introduce extensible load API")
 lib/bpf/bpf_validate.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/lib/bpf/bpf_validate.c b/lib/bpf/bpf_validate.c
index 5609bfcd5c16..6a2d5974e036 100644
--- a/lib/bpf/bpf_validate.c
+++ b/lib/bpf/bpf_validate.c
@@ -1220,10 +1220,11 @@ eval_max_load(struct bpf_reg_val *rv, uint64_t mask)
 	/* full 64-bit load */
 	if (mask == UINT64_MAX)
 		eval_smax_bound(rv, mask);
-
-	/* zero-extend load */
-	rv->s.min = rv->u.min;
-	rv->s.max = rv->u.max;
+	else {
+		/* zero-extend load */
+		rv->s.min = rv->u.min;
+		rv->s.max = rv->u.max;
+	}
 }
 
 
-- 
2.43.0


^ permalink raw reply related

* [PATCH v3 13/25] bpf/validate: fix BPF_MUL ranges minimum typo
From: Marat Khalili @ 2026-06-12 10:47 UTC (permalink / raw)
  To: Konstantin Ananyev; +Cc: dev, stable, Claudia Cauli
In-Reply-To: <20260612104743.6465-1-marat.khalili@huawei.com>

Function `eval_mul` calculated minimum of the both signed and unsigned
ranges as destination square instead of product with source due to a
typo.

E.g. consider the following program with the current validation code:

     Tested program:
         0:  mov r0, #0x0
         1:  ldxdw r2, [r1 + 0]
         2:  jlt r2, #0x11, L8
         3:  jgt r2, #0x1d, L8
         4:  jslt r2, #0x11, L8
         5:  jsgt r2, #0x1d, L8
         6:  mul r2, #0xb  ; tested instruction
         7:  mov r0, #0x1
         8:  exit
     Pre-state:
        r2:  17..29
     Post-state:
        r2:  289..319

After the tested instruction validator considers r2 to be no less than
289, however if 20 was loaded on step 1 it is possible for it after
multiplying by 11 to become 220 which is less than 289.

Fix the typo, add test.

Fixes: 8021917293d0 ("bpf: add extra validation for input BPF program")
Cc: stable@dpdk.org

Reported-by: Claudia Cauli <claudiacauli@gmail.com>
Signed-off-by: Marat Khalili <marat.khalili@huawei.com>
Acked-by: Konstantin Ananyev <konstantin.ananyev@huawei.com>
---
Depends-on: series-38434 ("bpf: introduce extensible load API")
 app/test/test_bpf_validate.c | 17 +++++++++++++++++
 lib/bpf/bpf_validate.c       |  4 ++--
 2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/app/test/test_bpf_validate.c b/app/test/test_bpf_validate.c
index 31a235a55af6..ecebfd48dd52 100644
--- a/app/test/test_bpf_validate.c
+++ b/app/test/test_bpf_validate.c
@@ -1289,6 +1289,23 @@ test_alu64_div_mod_overflow(void)
 REGISTER_FAST_TEST(bpf_validate_alu64_div_mod_overflow_autotest, NOHUGE_OK, ASAN_OK,
 	test_alu64_div_mod_overflow);
 
+/* 64-bit mul of small scalar range and immediate. */
+static int
+test_alu64_mul_k_range_small(void)
+{
+	return verify_instruction((struct verify_instruction_param){
+		.tested_instruction = {
+			.code = (EBPF_ALU64 | BPF_MUL | BPF_K),
+			.imm = 11,
+		},
+		.pre.dst = make_unsigned_domain(17, 29),
+		.post.dst = make_unsigned_domain(17 * 11, 29 * 11),
+	});
+}
+
+REGISTER_FAST_TEST(bpf_validate_alu64_mul_k_range_small_autotest, NOHUGE_OK, ASAN_OK,
+	test_alu64_mul_k_range_small);
+
 /* 64-bit negation when interval first element is INT64_MIN. */
 static int
 test_alu64_neg_int64min_first(void)
diff --git a/lib/bpf/bpf_validate.c b/lib/bpf/bpf_validate.c
index 14a186b7cbf7..08717671a863 100644
--- a/lib/bpf/bpf_validate.c
+++ b/lib/bpf/bpf_validate.c
@@ -915,7 +915,7 @@ eval_mul(struct bpf_reg_val *rd, const struct bpf_reg_val *rs, size_t opsz,
 	/* check for overflow */
 	} else if (rd->u.max <= msk >> opsz / 2 && rs->u.max <= msk >> opsz) {
 		rd->u.max *= rs->u.max;
-		rd->u.min *= rd->u.min;
+		rd->u.min *= rs->u.min;
 	} else
 		eval_umax_bound(rd, msk);
 
@@ -926,7 +926,7 @@ eval_mul(struct bpf_reg_val *rd, const struct bpf_reg_val *rs, size_t opsz,
 	/* check that both operands are positive and no overflow */
 	} else if (rd->s.min >= 0 && rs->s.min >= 0) {
 		rd->s.max *= rs->s.max;
-		rd->s.min *= rd->s.min;
+		rd->s.min *= rs->s.min;
 	} else
 		eval_smax_bound(rd, msk);
 }
-- 
2.43.0


^ permalink raw reply related

* [PATCH v3 14/25] bpf/validate: fix BPF_MUL signed overflow UB
From: Marat Khalili @ 2026-06-12 10:47 UTC (permalink / raw)
  To: Konstantin Ananyev; +Cc: dev, stable, Claudia Cauli
In-Reply-To: <20260612104743.6465-1-marat.khalili@huawei.com>

Function `eval_mul` triggered signed overflow for large constants.

E.g. consider the following program with the current validation code:

    Tested program:
        0:  mov r0, #0x0
        1:  lddw r2, #0x9876543210
        3:  mul r2, #0x12345678  ; tested instruction
        4:  mov r0, #0x1
        5:  exit

With sanitizer the following diagnostic is generated:

    lib/bpf/bpf_validate.c:1032:26: runtime error: signed integer
    overflow: 654820258320 * 305419896 cannot be represented in type
    'long int'
        #0 0x000002746bfd in eval_mul lib/bpf/bpf_validate.c:1032
        #1 0x00000274b6ac in eval_alu lib/bpf/bpf_validate.c:1260
        #2 0x00000275c526 in evaluate lib/bpf/bpf_validate.c:3174
        ...

    SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior
    lib/bpf/bpf_validate.c:1032:26

Multiply constants as unsigned which will produce mathematically correct
result in two's complement representation, add test.

Fixes: 8021917293d0 ("bpf: add extra validation for input BPF program")
Cc: stable@dpdk.org

Reported-by: Claudia Cauli <claudiacauli@gmail.com>
Signed-off-by: Marat Khalili <marat.khalili@huawei.com>
Acked-by: Konstantin Ananyev <konstantin.ananyev@huawei.com>
---
Depends-on: series-38434 ("bpf: introduce extensible load API")
 app/test/test_bpf_validate.c | 17 +++++++++++++++++
 lib/bpf/bpf_validate.c       |  4 ++--
 2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/app/test/test_bpf_validate.c b/app/test/test_bpf_validate.c
index ecebfd48dd52..15cdc83f4f14 100644
--- a/app/test/test_bpf_validate.c
+++ b/app/test/test_bpf_validate.c
@@ -1289,6 +1289,23 @@ test_alu64_div_mod_overflow(void)
 REGISTER_FAST_TEST(bpf_validate_alu64_div_mod_overflow_autotest, NOHUGE_OK, ASAN_OK,
 	test_alu64_div_mod_overflow);
 
+/* 64-bit multiplication of constant and immediate with overflow. */
+static int
+test_alu64_mul_k_overflow(void)
+{
+	return verify_instruction((struct verify_instruction_param){
+		.tested_instruction = {
+			.code = (EBPF_ALU64 | BPF_MUL | BPF_K),
+			.imm = 0x12345678,
+		},
+		.pre.dst = make_singleton_domain(0x9876543210),
+		.post.dst = make_singleton_domain(0x9876543210u * 0x12345678),
+	});
+}
+
+REGISTER_FAST_TEST(bpf_validate_alu64_mul_k_overflow_autotest, NOHUGE_OK, ASAN_OK,
+	test_alu64_mul_k_overflow);
+
 /* 64-bit mul of small scalar range and immediate. */
 static int
 test_alu64_mul_k_range_small(void)
diff --git a/lib/bpf/bpf_validate.c b/lib/bpf/bpf_validate.c
index 08717671a863..2b73e3628881 100644
--- a/lib/bpf/bpf_validate.c
+++ b/lib/bpf/bpf_validate.c
@@ -921,8 +921,8 @@ eval_mul(struct bpf_reg_val *rd, const struct bpf_reg_val *rs, size_t opsz,
 
 	/* both operands are constants */
 	if (rd->s.min == rd->s.max && rs->s.min == rs->s.max) {
-		rd->s.min = (rd->s.min * rs->s.min) & msk;
-		rd->s.max = (rd->s.max * rs->s.max) & msk;
+		rd->s.min = ((uint64_t)rd->s.min * (uint64_t)rs->s.min) & msk;
+		rd->s.max = ((uint64_t)rd->s.max * (uint64_t)rs->s.max) & msk;
 	/* check that both operands are positive and no overflow */
 	} else if (rd->s.min >= 0 && rs->s.min >= 0) {
 		rd->s.max *= rs->s.max;
-- 
2.43.0


^ permalink raw reply related

* [PATCH v3 15/25] bpf/validate: fix BPF_JGT/EBPF_JSGT no-jump max
From: Marat Khalili @ 2026-06-12 10:47 UTC (permalink / raw)
  To: Konstantin Ananyev; +Cc: dev, stable, Claudia Cauli
In-Reply-To: <20260612104743.6465-1-marat.khalili@huawei.com>

Functions `eval_jgt_jle` and `eval_jsgt_jsle` reduced range maximum for
BPF_JGT and EBPF_JSGT instructions in the no-jump case to the minimum of
src register instead of the maximum, producing more conservative
estimate that could cause false positives.

E.g. consider the following program with the current validation code:

    Tested program:
        0:  mov r0, #0x0
        1:  ldxdw r2, [r1 + 0]
        2:  jlt r2, #0x14, L15
        3:  jgt r2, #0x3c, L15
        4:  jslt r2, #0x14, L15
        5:  jsgt r2, #0x3c, L15
        6:  ldxdw r3, [r1 + 8]
        7:  jlt r3, #0x1e, L15
        8:  jgt r3, #0x32, L15
        9:  jslt r3, #0x1e, L15
       10:  jsgt r3, #0x32, L15
       11:  jgt r2, r3, L14  ; tested instruction
       12:  mov r0, #0x1
       13:  exit
       14:  mov r0, #0x2
       15:  exit
    Pre-state:
       r2:  20..60
       r3:  30..50
    Post-state:
       r2:  20..60 INTERSECT 0x14..0x1e (!)

Immediately after the tested instruction on step 12 validator expects r2
to contain values up to 60, for example 55, however for this value jump
condition r2 > r3 on step 11 would be always satisfied since r3 is known
to not exceed 50, and thus execution will always jump to step 14 instead
of continuing to step 12.

Fix range calculation, add tests for cases where range of src register
values is a strict subset of dst. Other cases will be covered in the
subsequent commits.

Fixes: 8021917293d0 ("bpf: add extra validation for input BPF program")
Cc: stable@dpdk.org

Reported-by: Claudia Cauli <claudiacauli@gmail.com>
Signed-off-by: Marat Khalili <marat.khalili@huawei.com>
Acked-by: Konstantin Ananyev <konstantin.ananyev@huawei.com>
---
Depends-on: series-38434 ("bpf: introduce extensible load API")
 app/test/test_bpf_validate.c | 90 ++++++++++++++++++++++++++++++++++++
 lib/bpf/bpf_validate.c       |  4 +-
 2 files changed, 92 insertions(+), 2 deletions(-)

diff --git a/app/test/test_bpf_validate.c b/app/test/test_bpf_validate.c
index 15cdc83f4f14..acc238f7d324 100644
--- a/app/test/test_bpf_validate.c
+++ b/app/test/test_bpf_validate.c
@@ -1485,6 +1485,96 @@ test_jmp64_jslt_x(void)
 REGISTER_FAST_TEST(bpf_validate_jmp64_jslt_x_autotest, NOHUGE_OK, ASAN_OK,
 	test_jmp64_jslt_x);
 
+/* Jump on ordering relationship with narrower range. */
+static int
+test_jmp64_jxx_x_ordering_narrower(void)
+{
+	TEST_ASSERT_SUCCESS(verify_instruction((struct verify_instruction_param){
+		.tested_instruction = {
+			.code = (BPF_JMP | BPF_JGT | BPF_X),
+		},
+		.pre.dst = make_signed_domain(20, 60),
+		.pre.src = make_signed_domain(30, 50),
+		.post.dst = make_signed_domain(20, 50),
+		.jump.dst = make_signed_domain(31, 60),
+	}), "(BPF_JMP | BPF_JGT | BPF_X) check");
+
+	TEST_ASSERT_SUCCESS(verify_instruction((struct verify_instruction_param){
+		.tested_instruction = {
+			.code = (BPF_JMP | BPF_JGE | BPF_X),
+		},
+		.pre.dst = make_signed_domain(20, 60),
+		.pre.src = make_signed_domain(30, 50),
+		.post.dst = make_signed_domain(20, 49),
+		.jump.dst = make_signed_domain(30, 60),
+	}), "(BPF_JMP | BPF_JGE | BPF_X) check");
+
+	TEST_ASSERT_SUCCESS(verify_instruction((struct verify_instruction_param){
+		.tested_instruction = {
+			.code = (BPF_JMP | EBPF_JLT | BPF_X),
+		},
+		.pre.dst = make_signed_domain(20, 60),
+		.pre.src = make_signed_domain(30, 50),
+		.post.dst = make_signed_domain(30, 60),
+		.jump.dst = make_signed_domain(20, 49),
+	}), "(BPF_JMP | EBPF_JLT | BPF_X) check");
+
+	TEST_ASSERT_SUCCESS(verify_instruction((struct verify_instruction_param){
+		.tested_instruction = {
+			.code = (BPF_JMP | EBPF_JLE | BPF_X),
+		},
+		.pre.dst = make_signed_domain(20, 60),
+		.pre.src = make_signed_domain(30, 50),
+		.post.dst = make_signed_domain(31, 60),
+		.jump.dst = make_signed_domain(20, 50),
+	}), "(BPF_JMP | EBPF_JLE | BPF_X) check");
+
+	TEST_ASSERT_SUCCESS(verify_instruction((struct verify_instruction_param){
+		.tested_instruction = {
+			.code = (BPF_JMP | EBPF_JSGT | BPF_X),
+		},
+		.pre.dst = make_signed_domain(20, 60),
+		.pre.src = make_signed_domain(30, 50),
+		.post.dst = make_signed_domain(20, 50),
+		.jump.dst = make_signed_domain(31, 60),
+	}), "(BPF_JMP | EBPF_JSGT | BPF_X) check");
+
+	TEST_ASSERT_SUCCESS(verify_instruction((struct verify_instruction_param){
+		.tested_instruction = {
+			.code = (BPF_JMP | EBPF_JSGE | BPF_X),
+		},
+		.pre.dst = make_signed_domain(20, 60),
+		.pre.src = make_signed_domain(30, 50),
+		.post.dst = make_signed_domain(20, 49),
+		.jump.dst = make_signed_domain(30, 60),
+	}), "(BPF_JMP | EBPF_JSGE | BPF_X) check");
+
+	TEST_ASSERT_SUCCESS(verify_instruction((struct verify_instruction_param){
+		.tested_instruction = {
+			.code = (BPF_JMP | EBPF_JSLT | BPF_X),
+		},
+		.pre.dst = make_signed_domain(20, 60),
+		.pre.src = make_signed_domain(30, 50),
+		.post.dst = make_signed_domain(30, 60),
+		.jump.dst = make_signed_domain(20, 49),
+	}), "(BPF_JMP | EBPF_JSLT | BPF_X) check");
+
+	TEST_ASSERT_SUCCESS(verify_instruction((struct verify_instruction_param){
+		.tested_instruction = {
+			.code = (BPF_JMP | EBPF_JSLE | BPF_X),
+		},
+		.pre.dst = make_signed_domain(20, 60),
+		.pre.src = make_signed_domain(30, 50),
+		.post.dst = make_signed_domain(31, 60),
+		.jump.dst = make_signed_domain(20, 50),
+	}), "(BPF_JMP | EBPF_JSLE | BPF_X) check");
+
+	return TEST_SUCCESS;
+}
+
+REGISTER_FAST_TEST(bpf_validate_jmp64_jxx_x_ordering_narrower_autotest, NOHUGE_OK, ASAN_OK,
+	test_jmp64_jxx_x_ordering_narrower);
+
 /* 64-bit load from heap (should be set to unknown). */
 static int
 test_mem_ldx_dw_heap(void)
diff --git a/lib/bpf/bpf_validate.c b/lib/bpf/bpf_validate.c
index 2b73e3628881..0578029dccbb 100644
--- a/lib/bpf/bpf_validate.c
+++ b/lib/bpf/bpf_validate.c
@@ -1521,7 +1521,7 @@ static void
 eval_jgt_jle(struct bpf_reg_val *trd, struct bpf_reg_val *trs,
 	struct bpf_reg_val *frd, struct bpf_reg_val *frs)
 {
-	frd->u.max = RTE_MIN(frd->u.max, frs->u.min);
+	frd->u.max = RTE_MIN(frd->u.max, frs->u.max);
 	trd->u.min = RTE_MAX(trd->u.min, trs->u.min + 1);
 }
 
@@ -1537,7 +1537,7 @@ static void
 eval_jsgt_jsle(struct bpf_reg_val *trd, struct bpf_reg_val *trs,
 	struct bpf_reg_val *frd, struct bpf_reg_val *frs)
 {
-	frd->s.max = RTE_MIN(frd->s.max, frs->s.min);
+	frd->s.max = RTE_MIN(frd->s.max, frs->s.max);
 	trd->s.min = RTE_MAX(trd->s.min, trs->s.min + 1);
 }
 
-- 
2.43.0


^ permalink raw reply related

* [PATCH v3 16/25] bpf/validate: fix BPF_JMP source range calculation
From: Marat Khalili @ 2026-06-12 10:47 UTC (permalink / raw)
  To: Konstantin Ananyev; +Cc: dev, stable, Claudia Cauli
In-Reply-To: <20260612104743.6465-1-marat.khalili@huawei.com>

All two-register ordering comparison functions (`eval_jgt_jle`,
`eval_jlt_jge`, `eval_jsgt_jsle`, `eval_jslt_jsge`) were updating only
the destination register value set but not the source register one. For
instance, instruction `jgt r2, r3` should be exactly equivalent to `jlt
r3, r2`, but previously the former only updated the possible values of
r2 while the latter only updated possible values of r3. Thus the
estimate for source register was conservative and could cause false
positives.

E.g. consider the following program with the current validation code:

    Tested program:
        0:  mov r0, #0x0
        1:  mov r2, #0x28
        2:  ldxdw r3, [r1 + 0]
        3:  jlt r3, #0x14, L11
        4:  jgt r3, #0x3c, L11
        5:  jslt r3, #0x14, L11
        6:  jsgt r3, #0x3c, L11
        7:  jgt r2, r3, L10  ; tested instruction
        8:  mov r0, #0x1
        9:  exit
       10:  mov r0, #0x2
       11:  exit
    Pre-state:
       r2:  40
       r3:  20..60
    ...
    Jump-state:
       r2:  40
       r3:  20..60

If tested instruction jumped from step 7 to step 10 validator expects r3
to contain values up to 60, for example 55, however for this value jump
condition r2 > r3 will never be satisfied since r2 is known to equal 40,
and thus execution would always continue to step 8 instead of jumping.

Add missing source register values update.

Introduce test harness for verifying all equivalent variations of a
comparison instruction. Add tests for all cases where both code branches
are reachable (unreachable branches will be covered by subsequent
commits).

Fixes: 8021917293d0 ("bpf: add extra validation for input BPF program")
Cc: stable@dpdk.org

Reported-by: Claudia Cauli <claudiacauli@gmail.com>
Signed-off-by: Marat Khalili <marat.khalili@huawei.com>
Acked-by: Konstantin Ananyev <konstantin.ananyev@huawei.com>
---
Depends-on: series-38434 ("bpf: introduce extensible load API")
 app/test/test_bpf_validate.c | 394 +++++++++++++++++++++++++++++++----
 lib/bpf/bpf_validate.c       |   8 +
 2 files changed, 358 insertions(+), 44 deletions(-)

diff --git a/app/test/test_bpf_validate.c b/app/test/test_bpf_validate.c
index acc238f7d324..15ccf4f6a573 100644
--- a/app/test/test_bpf_validate.c
+++ b/app/test/test_bpf_validate.c
@@ -32,6 +32,31 @@ RTE_LOG_REGISTER(test_bpf_validate_logtype, test.bpf_validate, NOTICE);
 #define REGISTER_FORMAT_BUFFER_SIZE 256
 #define DISASSEMBLY_FORMAT_BUFFER_SIZE 64
 
+#define COMPARISON_INDEX_IMMEDIATE RTE_BIT32(0)
+#define COMPARISON_INDEX_GREATER   RTE_BIT32(1)
+#define COMPARISON_INDEX_INCLUSIVE RTE_BIT32(2)
+#define COMPARISON_INDEX_SIGNED    RTE_BIT32(3)
+
+/* List comparison opcodes to make their index bits match constants above.  */
+static const uint8_t comparisons_opcode[] = {
+	(BPF_JMP | EBPF_JLT  | BPF_X),
+	(BPF_JMP | EBPF_JLT  | BPF_K),
+	(BPF_JMP |  BPF_JGT  | BPF_X),
+	(BPF_JMP |  BPF_JGT  | BPF_K),
+	(BPF_JMP | EBPF_JLE  | BPF_X),
+	(BPF_JMP | EBPF_JLE  | BPF_K),
+	(BPF_JMP |  BPF_JGE  | BPF_X),
+	(BPF_JMP |  BPF_JGE  | BPF_K),
+	(BPF_JMP | EBPF_JSLT | BPF_X),
+	(BPF_JMP | EBPF_JSLT | BPF_K),
+	(BPF_JMP | EBPF_JSGT | BPF_X),
+	(BPF_JMP | EBPF_JSGT | BPF_K),
+	(BPF_JMP | EBPF_JSLE | BPF_X),
+	(BPF_JMP | EBPF_JSLE | BPF_K),
+	(BPF_JMP | EBPF_JSGE | BPF_X),
+	(BPF_JMP | EBPF_JSGE | BPF_K),
+};
+
 /* Interval bounded by two signed values, inclusive; min <= max. */
 struct signed_interval {
 	int64_t min;
@@ -1044,6 +1069,206 @@ verify_instruction(struct verify_instruction_param prm)
 	return rc;
 }
 
+static int
+opcode_comparison_index(uint8_t opcode)
+{
+	for (int index = 0; index != RTE_DIM(comparisons_opcode); ++index)
+		if (comparisons_opcode[index] == opcode)
+			return index;
+	TEST_LOG_LINE(ERR, "Unsupported or not a comparison opcode: %hhx", opcode);
+	RTE_VERIFY(false);
+}
+
+/* Change two-register comparison verification to immediate one. */
+static bool
+make_comparison_immediate(struct verify_instruction_param *prm)
+{
+	int comparison_index = opcode_comparison_index(prm->tested_instruction.code);
+	const int64_t value = prm->pre.src.s.min;
+
+	if ((comparison_index & COMPARISON_INDEX_IMMEDIATE) != 0) {
+		TEST_LOG_LINE(ERR, "Comparison %hhx is already immediate.",
+			prm->tested_instruction.code);
+		RTE_VERIFY(false);
+	}
+
+	if (!domain_is_singleton(&prm->pre.src) || !domain_is_singleton(&prm->post.src) ||
+			!domain_is_singleton(&prm->jump.src)) {
+		TEST_LOG_LINE(DEBUG, "Cannot make immediate out of a non-singleton domain.");
+		return false;
+	}
+	if (prm->pre.src.is_pointer || prm->post.src.is_pointer || prm->jump.src.is_pointer) {
+		TEST_LOG_LINE(DEBUG, "Cannot make immediate out of a pointer.");
+		return false;
+	}
+	if (prm->post.src.s.min != value || prm->jump.src.s.min != value) {
+		TEST_LOG_LINE(DEBUG, "Cannot make immediate if the value changes.");
+		return false;
+	}
+	if (!fits_in_imm32(value)) {
+		TEST_LOG_LINE(ERR, "Cannot make immediate unless value fits in int32.");
+		return false;
+	}
+
+	comparison_index |= COMPARISON_INDEX_IMMEDIATE;
+	prm->tested_instruction.code = comparisons_opcode[comparison_index];
+	prm->tested_instruction.imm = value;
+
+	RTE_VERIFY(prm->pre.src.is_defined);
+	prm->pre.src.is_defined = false;
+
+	if (!prm->post.is_unreachable) {
+		RTE_VERIFY(prm->post.src.is_defined);
+		prm->post.src.is_defined = false;
+	}
+
+	if (!prm->jump.is_unreachable) {
+		RTE_VERIFY(prm->jump.src.is_defined);
+		prm->jump.src.is_defined = false;
+	}
+
+	return true;
+}
+
+/* Change immediate comparison verification to two-register one. */
+static void
+make_comparison_two_register(struct verify_instruction_param *prm)
+{
+	int comparison_index = opcode_comparison_index(prm->tested_instruction.code);
+	const int64_t value = prm->tested_instruction.imm;
+
+	if ((comparison_index & COMPARISON_INDEX_IMMEDIATE) == 0) {
+		TEST_LOG_LINE(ERR, "Comparison %hhx is already two-register.",
+			prm->tested_instruction.code);
+		RTE_VERIFY(false);
+	}
+
+	comparison_index &= ~COMPARISON_INDEX_IMMEDIATE;
+	prm->tested_instruction.code = comparisons_opcode[comparison_index];
+	prm->tested_instruction.imm = 0;
+
+	RTE_VERIFY(!prm->pre.src.is_defined);
+	prm->pre.src = make_singleton_domain(value);
+
+	if (!prm->post.is_unreachable) {
+		RTE_VERIFY(!prm->post.src.is_defined);
+		prm->post.src = prm->pre.src;
+	}
+
+	if (!prm->jump.is_unreachable) {
+		RTE_VERIFY(!prm->jump.src.is_defined);
+		prm->jump.src = prm->pre.src;
+	}
+}
+
+/* Change comparison verification to complement (negated result) one. */
+static void
+make_comparison_complement(struct verify_instruction_param *prm)
+{
+	int comparison_index = opcode_comparison_index(prm->tested_instruction.code);
+	comparison_index ^= COMPARISON_INDEX_GREATER | COMPARISON_INDEX_INCLUSIVE;
+	prm->tested_instruction.code = comparisons_opcode[comparison_index];
+	RTE_SWAP(prm->post, prm->jump);
+}
+
+/* Change comparison verification to converse (swapped operands) one. */
+static void
+make_comparison_converse(struct verify_instruction_param *prm)
+{
+	int comparison_index = opcode_comparison_index(prm->tested_instruction.code);
+	comparison_index ^= COMPARISON_INDEX_GREATER;
+	prm->tested_instruction.code = comparisons_opcode[comparison_index];
+	RTE_SWAP(prm->pre.dst, prm->pre.src);
+	RTE_SWAP(prm->post.dst, prm->post.src);
+	RTE_SWAP(prm->jump.dst, prm->jump.src);
+}
+
+/* Change signed comparison verification to unsigned one. */
+static void
+make_comparison_signed(struct verify_instruction_param *prm)
+{
+	int comparison_index = opcode_comparison_index(prm->tested_instruction.code);
+	if ((comparison_index & COMPARISON_INDEX_SIGNED) != 0) {
+		TEST_LOG_LINE(ERR, "Comparison %hhx is already signed.",
+			prm->tested_instruction.code);
+		RTE_VERIFY(false);
+	}
+	comparison_index |= COMPARISON_INDEX_SIGNED;
+	prm->tested_instruction.code = comparisons_opcode[comparison_index];
+}
+
+/* Verify specified two-register comparison and, if possible, immediate one. */
+static int
+verify_comparison_subcase(struct verify_instruction_param prm)
+{
+	TEST_ASSERT_SUCCESS(verify_instruction(prm), "two-register version check");
+
+	if (make_comparison_immediate(&prm))
+		TEST_ASSERT_SUCCESS(verify_instruction(prm), "immediate version check");
+
+	return TEST_SUCCESS;
+}
+
+/*
+ * Verify comparison instruction validation behaviour.
+ *
+ * Call `verify_instruction` for all valid variations of the instruction.
+ *
+ * For instance, `jgt r2, r3` verifies:
+ * * `jgt r2, r3`;
+ * * `jlt r3, r2` src and dst swapped with each other;
+ * * `jle r2, r3` with post and jump domains swapped with each other;
+ * * `jge r3, r2` with all corresponding swaps;
+ * * immediate versions of everything above where possible,
+ *   that is, register on the right is an int32 scalar singleton;
+ * * signed versions of everything above if `also_signed` is true;
+ *
+ * Regardless if passed instruction compares with immediate or singleton src
+ * both cases are generated and tested.
+ */
+static int
+verify_comparison(struct verify_instruction_param prm, bool also_signed)
+{
+	fill_verify_instruction_defaults(&prm);
+
+	if (!prm.pre.src.is_defined)
+		/* Convert from immediate form to simplify further logic. */
+		make_comparison_two_register(&prm);
+
+	/* All reachable domains must be defined by this point. */
+	RTE_VERIFY(prm.pre.dst.is_defined);
+	RTE_VERIFY(prm.pre.src.is_defined);
+	if (!prm.post.is_unreachable) {
+		RTE_VERIFY(prm.post.dst.is_defined);
+		RTE_VERIFY(prm.post.src.is_defined);
+	}
+	if (!prm.jump.is_unreachable) {
+		RTE_VERIFY(prm.jump.dst.is_defined);
+		RTE_VERIFY(prm.jump.src.is_defined);
+	}
+
+	for (int make_signed = 0; make_signed <= also_signed; ++make_signed) {
+		if (make_signed)
+			make_comparison_signed(&prm);
+
+		for (int complement = false; complement <= true; ++complement) {
+
+			for (int converse = false; converse <= true; ++converse) {
+
+				TEST_ASSERT_SUCCESS(verify_comparison_subcase(prm),
+					"make_signed=%d, complement=%d, converse=%d",
+					make_signed, complement, converse);
+
+				make_comparison_converse(&prm);
+			}
+
+			make_comparison_complement(&prm);
+		}
+	}
+
+	return TEST_SUCCESS;
+}
+
 
 /* TESTS FOR SPECIFIC INSTRUCTIONS */
 
@@ -1485,31 +1710,69 @@ test_jmp64_jslt_x(void)
 REGISTER_FAST_TEST(bpf_validate_jmp64_jslt_x_autotest, NOHUGE_OK, ASAN_OK,
 	test_jmp64_jslt_x);
 
-/* Jump on ordering relationship with narrower range. */
+/* Jump on ordering comparisons between two ranges. */
 static int
-test_jmp64_jxx_x_ordering_narrower(void)
+test_jmp64_ordering_ranges(void)
 {
-	TEST_ASSERT_SUCCESS(verify_instruction((struct verify_instruction_param){
+	/* All ranges used are valid for both signed and unsigned comparisons. */
+	const bool also_signed = true;
+
+	/*
+	 *     20 ---- dst ---- 60
+	 * 10 -- src -- 40
+	 */
+
+	TEST_ASSERT_SUCCESS(verify_comparison((struct verify_instruction_param){
 		.tested_instruction = {
-			.code = (BPF_JMP | BPF_JGT | BPF_X),
+			.code = (BPF_JMP | EBPF_JLT | BPF_X),
 		},
 		.pre.dst = make_signed_domain(20, 60),
-		.pre.src = make_signed_domain(30, 50),
-		.post.dst = make_signed_domain(20, 50),
-		.jump.dst = make_signed_domain(31, 60),
-	}), "(BPF_JMP | BPF_JGT | BPF_X) check");
+		.pre.src = make_signed_domain(10, 40),
+		.jump.dst = make_signed_domain(20, 39),
+		.jump.src = make_signed_domain(21, 40),
+	}, also_signed), "strict, dst range weakly greater than src range");
 
-	TEST_ASSERT_SUCCESS(verify_instruction((struct verify_instruction_param){
+	TEST_ASSERT_SUCCESS(verify_comparison((struct verify_instruction_param){
 		.tested_instruction = {
-			.code = (BPF_JMP | BPF_JGE | BPF_X),
+			.code = (BPF_JMP | EBPF_JLE | BPF_X),
 		},
 		.pre.dst = make_signed_domain(20, 60),
-		.pre.src = make_signed_domain(30, 50),
-		.post.dst = make_signed_domain(20, 49),
-		.jump.dst = make_signed_domain(30, 60),
-	}), "(BPF_JMP | BPF_JGE | BPF_X) check");
+		.pre.src = make_signed_domain(10, 40),
+		.jump.dst = make_signed_domain(20, 40),
+		.jump.src = make_signed_domain(20, 40),
+	}, also_signed), "non-strict, dst range weakly greater than src range");
 
-	TEST_ASSERT_SUCCESS(verify_instruction((struct verify_instruction_param){
+	/*
+	 *     20 ---- dst ---- 60
+	 * 10 -------- src -------- 70
+	 */
+
+	TEST_ASSERT_SUCCESS(verify_comparison((struct verify_instruction_param){
+		.tested_instruction = {
+			.code = (BPF_JMP | EBPF_JLT | BPF_X),
+		},
+		.pre.dst = make_signed_domain(20, 60),
+		.pre.src = make_signed_domain(10, 70),
+		.post.src = make_signed_domain(10, 60),
+		.jump.src = make_signed_domain(21, 70),
+	}, also_signed), "strict, dst range included in src range");
+
+	TEST_ASSERT_SUCCESS(verify_comparison((struct verify_instruction_param){
+		.tested_instruction = {
+			.code = (BPF_JMP | EBPF_JLE | BPF_X),
+		},
+		.pre.dst = make_signed_domain(20, 60),
+		.pre.src = make_signed_domain(10, 70),
+		.post.src = make_signed_domain(10, 59),
+		.jump.src = make_signed_domain(20, 70),
+	}, also_signed), "non-strict, dst range included in src range");
+
+	/*
+	 *     20 ---- dst ---- 60
+	 *        30 - src - 50
+	 */
+
+	TEST_ASSERT_SUCCESS(verify_comparison((struct verify_instruction_param){
 		.tested_instruction = {
 			.code = (BPF_JMP | EBPF_JLT | BPF_X),
 		},
@@ -1517,9 +1780,9 @@ test_jmp64_jxx_x_ordering_narrower(void)
 		.pre.src = make_signed_domain(30, 50),
 		.post.dst = make_signed_domain(30, 60),
 		.jump.dst = make_signed_domain(20, 49),
-	}), "(BPF_JMP | EBPF_JLT | BPF_X) check");
+	}, also_signed), "strict, dst range includes src range");
 
-	TEST_ASSERT_SUCCESS(verify_instruction((struct verify_instruction_param){
+	TEST_ASSERT_SUCCESS(verify_comparison((struct verify_instruction_param){
 		.tested_instruction = {
 			.code = (BPF_JMP | EBPF_JLE | BPF_X),
 		},
@@ -1527,53 +1790,96 @@ test_jmp64_jxx_x_ordering_narrower(void)
 		.pre.src = make_signed_domain(30, 50),
 		.post.dst = make_signed_domain(31, 60),
 		.jump.dst = make_signed_domain(20, 50),
-	}), "(BPF_JMP | EBPF_JLE | BPF_X) check");
+	}, also_signed), "non-strict, dst range includes src range");
 
-	TEST_ASSERT_SUCCESS(verify_instruction((struct verify_instruction_param){
+	/*
+	 *     20 ---- dst ---- 60
+	 *             40 -- src -- 70
+	 */
+
+	TEST_ASSERT_SUCCESS(verify_comparison((struct verify_instruction_param){
 		.tested_instruction = {
-			.code = (BPF_JMP | EBPF_JSGT | BPF_X),
+			.code = (BPF_JMP | EBPF_JLT | BPF_X),
 		},
 		.pre.dst = make_signed_domain(20, 60),
-		.pre.src = make_signed_domain(30, 50),
-		.post.dst = make_signed_domain(20, 50),
-		.jump.dst = make_signed_domain(31, 60),
-	}), "(BPF_JMP | EBPF_JSGT | BPF_X) check");
+		.pre.src = make_signed_domain(40, 70),
+		.post.dst = make_signed_domain(40, 60),
+		.post.src = make_signed_domain(40, 60),
+	}, also_signed), "strict, dst range weakly less than src range");
 
-	TEST_ASSERT_SUCCESS(verify_instruction((struct verify_instruction_param){
+	TEST_ASSERT_SUCCESS(verify_comparison((struct verify_instruction_param){
 		.tested_instruction = {
-			.code = (BPF_JMP | EBPF_JSGE | BPF_X),
+			.code = (BPF_JMP | EBPF_JLE | BPF_X),
 		},
 		.pre.dst = make_signed_domain(20, 60),
-		.pre.src = make_signed_domain(30, 50),
-		.post.dst = make_signed_domain(20, 49),
-		.jump.dst = make_signed_domain(30, 60),
-	}), "(BPF_JMP | EBPF_JSGE | BPF_X) check");
+		.pre.src = make_signed_domain(40, 70),
+		.post.dst = make_signed_domain(41, 60),
+		.post.src = make_signed_domain(40, 59),
+	}, also_signed), "non-strict, dst range weakly less than src range");
 
-	TEST_ASSERT_SUCCESS(verify_instruction((struct verify_instruction_param){
+	return TEST_SUCCESS;
+}
+
+REGISTER_FAST_TEST(bpf_validate_jmp64_ordering_ranges_autotest, NOHUGE_OK, ASAN_OK,
+	test_jmp64_ordering_ranges);
+
+/* Jump on ordering comparisons with singleton. */
+static int
+test_jmp64_ordering_singleton(void)
+{
+	/* All ranges used are valid for both signed and unsigned comparisons. */
+	const bool also_signed = true;
+
+	/*
+	 *     20 ---- dst ---- 60
+	 *             imm
+	 */
+
+	TEST_ASSERT_SUCCESS(verify_comparison((struct verify_instruction_param){
 		.tested_instruction = {
-			.code = (BPF_JMP | EBPF_JSLT | BPF_X),
+			.code = (BPF_JMP | EBPF_JLT | BPF_K),
+			.imm = 40,
 		},
 		.pre.dst = make_signed_domain(20, 60),
-		.pre.src = make_signed_domain(30, 50),
-		.post.dst = make_signed_domain(30, 60),
-		.jump.dst = make_signed_domain(20, 49),
-	}), "(BPF_JMP | EBPF_JSLT | BPF_X) check");
+		.post.dst = make_signed_domain(40, 60),
+		.jump.dst = make_signed_domain(20, 39),
+	}, also_signed), "(BPF_JMP | EBPF_JLT | BPF_K) check");
 
-	TEST_ASSERT_SUCCESS(verify_instruction((struct verify_instruction_param){
+	TEST_ASSERT_SUCCESS(verify_comparison((struct verify_instruction_param){
 		.tested_instruction = {
-			.code = (BPF_JMP | EBPF_JSLE | BPF_X),
+			.code = (BPF_JMP | BPF_JGT | BPF_K),
+			.imm = 40,
 		},
 		.pre.dst = make_signed_domain(20, 60),
-		.pre.src = make_signed_domain(30, 50),
-		.post.dst = make_signed_domain(31, 60),
-		.jump.dst = make_signed_domain(20, 50),
-	}), "(BPF_JMP | EBPF_JSLE | BPF_X) check");
+		.post.dst = make_signed_domain(20, 40),
+		.jump.dst = make_signed_domain(41, 60),
+	}, also_signed), "(BPF_JMP | EBPF_JGT | BPF_K) check");
+
+	TEST_ASSERT_SUCCESS(verify_comparison((struct verify_instruction_param){
+		.tested_instruction = {
+			.code = (BPF_JMP | EBPF_JLE | BPF_K),
+			.imm = 40,
+		},
+		.pre.dst = make_signed_domain(20, 60),
+		.post.dst = make_signed_domain(41, 60),
+		.jump.dst = make_signed_domain(20, 40),
+	}, also_signed), "(BPF_JMP | EBPF_JLE | BPF_K) check");
+
+	TEST_ASSERT_SUCCESS(verify_comparison((struct verify_instruction_param){
+		.tested_instruction = {
+			.code = (BPF_JMP | BPF_JGE | BPF_K),
+			.imm = 40,
+		},
+		.pre.dst = make_signed_domain(20, 60),
+		.post.dst = make_signed_domain(20, 39),
+		.jump.dst = make_signed_domain(40, 60),
+	}, also_signed), "(BPF_JMP | EBPF_JGE | BPF_K) check");
 
 	return TEST_SUCCESS;
 }
 
-REGISTER_FAST_TEST(bpf_validate_jmp64_jxx_x_ordering_narrower_autotest, NOHUGE_OK, ASAN_OK,
-	test_jmp64_jxx_x_ordering_narrower);
+REGISTER_FAST_TEST(bpf_validate_jmp64_ordering_singleton_autotest, NOHUGE_OK, ASAN_OK,
+	test_jmp64_ordering_singleton);
 
 /* 64-bit load from heap (should be set to unknown). */
 static int
diff --git a/lib/bpf/bpf_validate.c b/lib/bpf/bpf_validate.c
index 0578029dccbb..2e535069fe4d 100644
--- a/lib/bpf/bpf_validate.c
+++ b/lib/bpf/bpf_validate.c
@@ -1522,7 +1522,9 @@ eval_jgt_jle(struct bpf_reg_val *trd, struct bpf_reg_val *trs,
 	struct bpf_reg_val *frd, struct bpf_reg_val *frs)
 {
 	frd->u.max = RTE_MIN(frd->u.max, frs->u.max);
+	frs->u.min = RTE_MAX(frs->u.min, frd->u.min);
 	trd->u.min = RTE_MAX(trd->u.min, trs->u.min + 1);
+	trs->u.max = RTE_MIN(trs->u.max, trd->u.max - 1);
 }
 
 static void
@@ -1530,7 +1532,9 @@ eval_jlt_jge(struct bpf_reg_val *trd, struct bpf_reg_val *trs,
 	struct bpf_reg_val *frd, struct bpf_reg_val *frs)
 {
 	frd->u.min = RTE_MAX(frd->u.min, frs->u.min);
+	frs->u.max = RTE_MIN(frs->u.max, frd->u.max);
 	trd->u.max = RTE_MIN(trd->u.max, trs->u.max - 1);
+	trs->u.min = RTE_MAX(trs->u.min, trd->u.min + 1);
 }
 
 static void
@@ -1538,7 +1542,9 @@ eval_jsgt_jsle(struct bpf_reg_val *trd, struct bpf_reg_val *trs,
 	struct bpf_reg_val *frd, struct bpf_reg_val *frs)
 {
 	frd->s.max = RTE_MIN(frd->s.max, frs->s.max);
+	frs->s.min = RTE_MAX(frs->s.min, frd->s.min);
 	trd->s.min = RTE_MAX(trd->s.min, trs->s.min + 1);
+	trs->s.max = RTE_MIN(trs->s.max, trd->s.max - 1);
 }
 
 static void
@@ -1546,7 +1552,9 @@ eval_jslt_jsge(struct bpf_reg_val *trd, struct bpf_reg_val *trs,
 	struct bpf_reg_val *frd, struct bpf_reg_val *frs)
 {
 	frd->s.min = RTE_MAX(frd->s.min, frs->s.min);
+	frs->s.max = RTE_MIN(frs->s.max, frd->s.max);
 	trd->s.max = RTE_MIN(trd->s.max, trs->s.max - 1);
+	trs->s.min = RTE_MAX(trs->s.min, trd->s.min + 1);
 }
 
 static const char *
-- 
2.43.0


^ permalink raw reply related

* [PATCH v3 17/25] bpf/validate: fix BPF_JMP empty range handling
From: Marat Khalili @ 2026-06-12 10:47 UTC (permalink / raw)
  To: Konstantin Ananyev; +Cc: dev, stable, Claudia Cauli
In-Reply-To: <20260612104743.6465-1-marat.khalili@huawei.com>

Function `eval_jcc` did not account for 'dynamically unreachable' code
paths. Some code paths may be _dynamically_ unreachable, which measn
that according to validator calculations no valid values are left to
evaluate. This does not indicate dead code since same code might be
reachable through other code paths. Previous behaviour resulted in:
* undefined behaviour in corner cases;
* ranges breaking min <= max invariant relied upon in multiple places
  (e.g. signed overflow detection in `eval_mul` only checks `s.min` to
  make sure the range is non-negative and so on);
* unnecessary work for validator contributing to exponential code paths
  grow in some cases.

E.g. consider the following program with the current validation code:

    Tested program:
        0:  mov r0, #0x0
        1:  mov r2, #0x2a
        2:  lddw r3, #0x8000000000000000
        4:  jslt r2, r3, L7  ; tested instruction
        5:  mov r0, #0x1
        6:  exit
        7:  mov r0, #0x2
        8:  exit
    Pre-state:
       r2:  42
       r3:  INT64_MIN
    Post-state:
       r2:  42
       r3:  INT64_MIN
    Jump-state:
       r2:  42
       r3:  43..INT64_MIN INTERSECT 0x8000000000000000 (!)

At step 7 after jump from tested instruction validator considers r3 to
equal 0x8000000000000000 if viewed as unsigned, or have nonsensical
range 43..INT64_MIN if viewed as signed. In reality there is just no
valid range for this code path since it will never occur.

With sanitizer the following diagnostic is generated:

    lib/bpf/bpf_validate.c:1824:15: runtime error: signed integer
    overflow: -9223372036854775808 - 1 cannot be represented in type
    'long int'
        #0 0x000002761e41 in eval_jslt_jsge lib/bpf/bpf_validate.c:1824
        #1 0x000002762acb in eval_jcc lib/bpf/bpf_validate.c:1881
        #2 0x00000276b749 in evaluate lib/bpf/bpf_validate.c:3245
    ...

    SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior
    lib/bpf/bpf_validate.c:1824:15

Add pruning of dynamically unreachable code paths that arise from
ordering comparisons. Add tests for remaining ordering jump cases.

Fixes: 8021917293d0 ("bpf: add extra validation for input BPF program")
Cc: stable@dpdk.org

Reported-by: Claudia Cauli <claudiacauli@gmail.com>
Signed-off-by: Marat Khalili <marat.khalili@huawei.com>
Acked-by: Konstantin Ananyev <konstantin.ananyev@huawei.com>
---
Depends-on: series-38434 ("bpf: introduce extensible load API")
 app/test/test_bpf_validate.c     | 277 ++++++++++++++++++++++++++++++-
 lib/bpf/bpf_validate.c           |  96 ++++++++---
 lib/bpf/rte_bpf_validate_debug.h |   2 +
 3 files changed, 351 insertions(+), 24 deletions(-)

diff --git a/app/test/test_bpf_validate.c b/app/test/test_bpf_validate.c
index 15ccf4f6a573..f80dee24a677 100644
--- a/app/test/test_bpf_validate.c
+++ b/app/test/test_bpf_validate.c
@@ -135,6 +135,11 @@ static const struct domain unknown = {
 	.u = { .min = 0, .max = UINT64_MAX },
 };
 
+/* Unreachable state. */
+static const struct state unreachable = {
+	.is_unreachable = true,
+};
+
 
 /* BUILDING DOMAINS */
 
@@ -1710,6 +1715,55 @@ test_jmp64_jslt_x(void)
 REGISTER_FAST_TEST(bpf_validate_jmp64_jslt_x_autotest, NOHUGE_OK, ASAN_OK,
 	test_jmp64_jslt_x);
 
+/* Jump on ordering comparisons with potential bound overflow. */
+static int
+test_jmp64_ordering_overflow(void)
+{
+	/* In this test signed and unsigned cases are spelled out explicitly. */
+	const bool also_signed = false;
+
+	TEST_ASSERT_SUCCESS(verify_comparison((struct verify_instruction_param){
+		.tested_instruction = {
+			.code = (BPF_JMP | EBPF_JSLT | BPF_X),
+		},
+		.pre.dst = make_singleton_domain(42),
+		.pre.src = make_singleton_domain(INT64_MIN),
+		.jump = unreachable,
+	}, also_signed), "signed less than INT64_MIN");
+
+	TEST_ASSERT_SUCCESS(verify_comparison((struct verify_instruction_param){
+		.tested_instruction = {
+			.code = (BPF_JMP | EBPF_JSGT | BPF_X),
+		},
+		.pre.dst = make_singleton_domain(42),
+		.pre.src = make_singleton_domain(INT64_MAX),
+		.jump = unreachable,
+	}, also_signed), "signed greater than INT64_MAX");
+
+	TEST_ASSERT_SUCCESS(verify_comparison((struct verify_instruction_param){
+		.tested_instruction = {
+			.code = (BPF_JMP | EBPF_JLT | BPF_X),
+		},
+		.pre.dst = make_singleton_domain(42),
+		.pre.src = make_singleton_domain(0),
+		.jump = unreachable,
+	}, also_signed), "unsigned less than zero");
+
+	TEST_ASSERT_SUCCESS(verify_comparison((struct verify_instruction_param){
+		.tested_instruction = {
+			.code = (BPF_JMP | BPF_JGT | BPF_X),
+		},
+		.pre.dst = make_singleton_domain(42),
+		.pre.src = make_singleton_domain(UINT64_MAX),
+		.jump = unreachable,
+	}, also_signed), "unsigned greater than UINT64_MAX");
+
+	return TEST_SUCCESS;
+}
+
+REGISTER_FAST_TEST(bpf_validate_jmp64_ordering_overflow_autotest, NOHUGE_OK, ASAN_OK,
+	test_jmp64_ordering_overflow);
+
 /* Jump on ordering comparisons between two ranges. */
 static int
 test_jmp64_ordering_ranges(void)
@@ -1717,6 +1771,29 @@ test_jmp64_ordering_ranges(void)
 	/* All ranges used are valid for both signed and unsigned comparisons. */
 	const bool also_signed = true;
 
+	/*
+	 *               20 ---- dst ---- 60
+	 * 0 - src - 10
+	 */
+
+	TEST_ASSERT_SUCCESS(verify_comparison((struct verify_instruction_param){
+		.tested_instruction = {
+			.code = (BPF_JMP | EBPF_JLT | BPF_X),
+		},
+		.pre.dst = make_signed_domain(20, 60),
+		.pre.src = make_signed_domain(0, 10),
+		.jump = unreachable,
+	}, also_signed), "strict, dst range strongly greater than src range");
+
+	TEST_ASSERT_SUCCESS(verify_comparison((struct verify_instruction_param){
+		.tested_instruction = {
+			.code = (BPF_JMP | EBPF_JLE | BPF_X),
+		},
+		.pre.dst = make_signed_domain(20, 60),
+		.pre.src = make_signed_domain(0, 10),
+		.jump = unreachable,
+	}, also_signed), "non-strict, dst range strongly greater than src range");
+
 	/*
 	 *     20 ---- dst ---- 60
 	 * 10 -- src -- 40
@@ -1817,15 +1894,38 @@ test_jmp64_ordering_ranges(void)
 		.post.src = make_signed_domain(40, 59),
 	}, also_signed), "non-strict, dst range weakly less than src range");
 
+	/*
+	 *     20 ---- dst ---- 60
+	 *                          70 - src - 80
+	 */
+
+	TEST_ASSERT_SUCCESS(verify_comparison((struct verify_instruction_param){
+		.tested_instruction = {
+			.code = (BPF_JMP | EBPF_JLT | BPF_X),
+		},
+		.pre.dst = make_signed_domain(20, 60),
+		.pre.src = make_signed_domain(70, 80),
+		.post = unreachable,
+	}, also_signed), "strict, dst range strongly less than src range");
+
+	TEST_ASSERT_SUCCESS(verify_comparison((struct verify_instruction_param){
+		.tested_instruction = {
+			.code = (BPF_JMP | EBPF_JLE | BPF_X),
+		},
+		.pre.dst = make_signed_domain(20, 60),
+		.pre.src = make_signed_domain(70, 80),
+		.post = unreachable,
+	}, also_signed), "non-strict, dst range strongly less than src range");
+
 	return TEST_SUCCESS;
 }
 
 REGISTER_FAST_TEST(bpf_validate_jmp64_ordering_ranges_autotest, NOHUGE_OK, ASAN_OK,
 	test_jmp64_ordering_ranges);
 
-/* Jump on ordering comparisons with singleton. */
+/* Jump on ordering comparisons with singleton inside the range. */
 static int
-test_jmp64_ordering_singleton(void)
+test_jmp64_ordering_singleton_inside(void)
 {
 	/* All ranges used are valid for both signed and unsigned comparisons. */
 	const bool also_signed = true;
@@ -1878,8 +1978,177 @@ test_jmp64_ordering_singleton(void)
 	return TEST_SUCCESS;
 }
 
-REGISTER_FAST_TEST(bpf_validate_jmp64_ordering_singleton_autotest, NOHUGE_OK, ASAN_OK,
-	test_jmp64_ordering_singleton);
+REGISTER_FAST_TEST(bpf_validate_jmp64_ordering_singleton_inside_autotest, NOHUGE_OK, ASAN_OK,
+	test_jmp64_ordering_singleton_inside);
+
+/* Jump on ordering comparisons with singleton outside the range. */
+static int
+test_jmp64_ordering_singleton_outside(void)
+{
+	/* All ranges used are valid for both signed and unsigned comparisons. */
+	const bool also_signed = true;
+
+	/*
+	 *       20 ---- dst ---- 60
+	 *  imm
+	 */
+
+	TEST_ASSERT_SUCCESS(verify_comparison((struct verify_instruction_param){
+		.tested_instruction = {
+			.code = (BPF_JMP | EBPF_JLT | BPF_K),
+			.imm = 10,
+		},
+		.pre.dst = make_signed_domain(20, 60),
+		.jump = unreachable,
+	}, also_signed), "(BPF_JMP | EBPF_JLT | BPF_K) check, range greater than imm");
+
+	TEST_ASSERT_SUCCESS(verify_comparison((struct verify_instruction_param){
+		.tested_instruction = {
+			.code = (BPF_JMP | EBPF_JLE | BPF_K),
+			.imm = 10,
+		},
+		.pre.dst = make_signed_domain(20, 60),
+		.jump = unreachable,
+	}, also_signed), "(BPF_JMP | EBPF_JLE | BPF_K) check, range greater than imm");
+
+	TEST_ASSERT_SUCCESS(verify_comparison((struct verify_instruction_param){
+		.tested_instruction = {
+			.code = (BPF_JMP | BPF_JGT | BPF_K),
+			.imm = 10,
+		},
+		.pre.dst = make_signed_domain(20, 60),
+		.post = unreachable,
+	}, also_signed), "(BPF_JMP | EBPF_JGT | BPF_K) check, range greater than imm");
+
+	TEST_ASSERT_SUCCESS(verify_comparison((struct verify_instruction_param){
+		.tested_instruction = {
+			.code = (BPF_JMP | BPF_JGE | BPF_K),
+			.imm = 10,
+		},
+		.pre.dst = make_signed_domain(20, 60),
+		.post = unreachable,
+	}, also_signed), "(BPF_JMP | EBPF_JGE | BPF_K) check, range greater than imm");
+
+	/*
+	 *       20 ---- dst ---- 60
+	 *                            imm
+	 */
+
+	TEST_ASSERT_SUCCESS(verify_comparison((struct verify_instruction_param){
+		.tested_instruction = {
+			.code = (BPF_JMP | EBPF_JLT | BPF_K),
+			.imm = 70,
+		},
+		.pre.dst = make_signed_domain(20, 60),
+		.post = unreachable,
+	}, also_signed), "(BPF_JMP | EBPF_JLT | BPF_K) check, range less than imm");
+
+	TEST_ASSERT_SUCCESS(verify_comparison((struct verify_instruction_param){
+		.tested_instruction = {
+			.code = (BPF_JMP | EBPF_JLE | BPF_K),
+			.imm = 70,
+		},
+		.pre.dst = make_signed_domain(20, 60),
+		.post = unreachable,
+	}, also_signed), "(BPF_JMP | EBPF_JLE | BPF_K) check, range less than imm");
+
+	TEST_ASSERT_SUCCESS(verify_comparison((struct verify_instruction_param){
+		.tested_instruction = {
+			.code = (BPF_JMP | BPF_JGT | BPF_K),
+			.imm = 70,
+		},
+		.pre.dst = make_signed_domain(20, 60),
+		.jump = unreachable,
+	}, also_signed), "(BPF_JMP | EBPF_JGT | BPF_K) check, range less than imm");
+
+	TEST_ASSERT_SUCCESS(verify_comparison((struct verify_instruction_param){
+		.tested_instruction = {
+			.code = (BPF_JMP | BPF_JGE | BPF_K),
+			.imm = 70,
+		},
+		.pre.dst = make_signed_domain(20, 60),
+		.jump = unreachable,
+	}, also_signed), "(BPF_JMP | EBPF_JGE | BPF_K) check, range less than imm");
+
+	return TEST_SUCCESS;
+}
+
+REGISTER_FAST_TEST(bpf_validate_jmp64_ordering_singleton_outside_autotest, NOHUGE_OK, ASAN_OK,
+	test_jmp64_ordering_singleton_outside);
+
+/* Jump on ordering comparisons with ranges "touching" each other. */
+static int
+test_jmp64_ordering_touching(void)
+{
+	/* All ranges used are valid for both signed and unsigned comparisons. */
+	const bool also_signed = true;
+
+	for (int overlap = 0; overlap != 3; ++overlap) {
+
+		/*
+		 *                  20 - dst - 30
+		 * 10 - src - (19 + overlap)
+		 */
+
+		TEST_ASSERT_SUCCESS(verify_comparison((struct verify_instruction_param){
+			.tested_instruction = {
+				.code = (BPF_JMP | EBPF_JLT | BPF_X),
+			},
+			.pre.dst = make_signed_domain(20, 30),
+			.pre.src = make_signed_domain(10, 19 + overlap),
+			.jump = overlap <= 1 ? unreachable : (struct state){
+				.dst = make_singleton_domain(20),
+				.src = make_singleton_domain(21),
+			},
+		}, also_signed), "strict, dst left touching src right, overlap=%d", overlap);
+
+		TEST_ASSERT_SUCCESS(verify_comparison((struct verify_instruction_param){
+			.tested_instruction = {
+				.code = (BPF_JMP | EBPF_JLE | BPF_X),
+			},
+			.pre.dst = make_signed_domain(20, 30),
+			.pre.src = make_signed_domain(10, 19 + overlap),
+			.jump = overlap < 1 ? unreachable : (struct state){
+				.dst = make_signed_domain(20, 19 + overlap),
+				.src = make_signed_domain(20, 19 + overlap),
+			},
+		}, also_signed), "non-strict, dst left touching src right, overlap=%d", overlap);
+
+		/*
+		 * 10 - dst - (19 + overlap)
+		 *                  20 - src - 30
+		 */
+
+		TEST_ASSERT_SUCCESS(verify_comparison((struct verify_instruction_param){
+			.tested_instruction = {
+				.code = (BPF_JMP | EBPF_JLT | BPF_X),
+			},
+			.pre.dst = make_signed_domain(10, 19 + overlap),
+			.pre.src = make_signed_domain(20, 30),
+			.post = overlap < 1 ? unreachable : (struct state){
+				.dst = make_signed_domain(20, 19 + overlap),
+				.src = make_signed_domain(20, 19 + overlap),
+			},
+		}, also_signed), "strict, dst right touching src left, overlap=%d", overlap);
+
+		TEST_ASSERT_SUCCESS(verify_comparison((struct verify_instruction_param){
+			.tested_instruction = {
+				.code = (BPF_JMP | EBPF_JLE | BPF_X),
+			},
+			.pre.dst = make_signed_domain(10, 19 + overlap),
+			.pre.src = make_signed_domain(20, 30),
+			.post = overlap <= 1 ? unreachable : (struct state){
+				.dst = make_singleton_domain(21),
+				.src = make_singleton_domain(20),
+			},
+		}, also_signed), "non-strict, dst right touching src left, overlap=%d", overlap);
+	}
+
+	return TEST_SUCCESS;
+}
+
+REGISTER_FAST_TEST(bpf_validate_jmp64_ordering_touching_autotest, NOHUGE_OK, ASAN_OK,
+	test_jmp64_ordering_touching);
 
 /* 64-bit load from heap (should be set to unknown). */
 static int
diff --git a/lib/bpf/bpf_validate.c b/lib/bpf/bpf_validate.c
index 2e535069fe4d..af084e36c8d0 100644
--- a/lib/bpf/bpf_validate.c
+++ b/lib/bpf/bpf_validate.c
@@ -19,6 +19,9 @@
 
 #define BPF_ARG_PTR_STACK RTE_BPF_ARG_RESERVED
 
+/* type containing no values (AKA "bottom", "never" etc)  */
+#define BPF_ARG_UNINHABITED ((enum rte_bpf_arg_type)(RTE_BPF_ARG_UNDEF - 1))
+
 struct bpf_reg_val {
 	struct rte_bpf_arg v;
 	uint64_t mask;
@@ -36,6 +39,8 @@ struct bpf_eval_state {
 	SLIST_ENTRY(bpf_eval_state) next; /* for @safe list traversal */
 	struct bpf_reg_val rv[EBPF_REG_NUM];
 	struct bpf_reg_val sv[MAX_BPF_STACK_SIZE / sizeof(uint64_t)];
+	/* flag set for branches determined to be dynamically unreachable */
+	bool unreachable;
 };
 
 SLIST_HEAD(bpf_evst_head, bpf_eval_state);
@@ -174,6 +179,9 @@ __rte_bpf_validate_can_access(const struct bpf_verifier *verifier,
 	struct value_set access_set;
 	uint32_t opsz;
 
+	if (st->unreachable)
+		return -ENOENT;
+
 	switch (BPF_CLASS(access->code)) {
 	case BPF_LDX:
 		rv = &st->rv[access->src_reg];
@@ -310,6 +318,10 @@ __rte_bpf_validate_may_jump(const struct bpf_verifier *verifier,
 	if (!may_jump_code_is_supported(jump->code))
 		return -ENOTSUP;
 
+	if (st->unreachable)
+		/* Set no bits since neither false nor true is possible. */
+		return 0;
+
 	rd = &st->rv[jump->dst_reg];
 	dst_set = (rd->v.type == RTE_BPF_ARG_UNDEF) ? value_set_full :
 		value_set_from_pair(rd->s.min, rd->s.max, rd->u.min, rd->u.max);
@@ -1521,40 +1533,68 @@ static void
 eval_jgt_jle(struct bpf_reg_val *trd, struct bpf_reg_val *trs,
 	struct bpf_reg_val *frd, struct bpf_reg_val *frs)
 {
-	frd->u.max = RTE_MIN(frd->u.max, frs->u.max);
-	frs->u.min = RTE_MAX(frs->u.min, frd->u.min);
-	trd->u.min = RTE_MAX(trd->u.min, trs->u.min + 1);
-	trs->u.max = RTE_MIN(trs->u.max, trd->u.max - 1);
+	if (frd->u.min <= frs->u.max) {
+		frd->u.max = RTE_MIN(frd->u.max, frs->u.max);
+		frs->u.min = RTE_MAX(frs->u.min, frd->u.min);
+	} else
+		frd->v.type = frs->v.type = BPF_ARG_UNINHABITED;
+
+	if (trs->u.min < trd->u.max) {
+		trd->u.min = RTE_MAX(trd->u.min, trs->u.min + 1);
+		trs->u.max = RTE_MIN(trs->u.max, trd->u.max - 1);
+	} else
+		trd->v.type = trs->v.type = BPF_ARG_UNINHABITED;
 }
 
 static void
 eval_jlt_jge(struct bpf_reg_val *trd, struct bpf_reg_val *trs,
 	struct bpf_reg_val *frd, struct bpf_reg_val *frs)
 {
-	frd->u.min = RTE_MAX(frd->u.min, frs->u.min);
-	frs->u.max = RTE_MIN(frs->u.max, frd->u.max);
-	trd->u.max = RTE_MIN(trd->u.max, trs->u.max - 1);
-	trs->u.min = RTE_MAX(trs->u.min, trd->u.min + 1);
+	if (frs->u.min <= frd->u.max) {
+		frd->u.min = RTE_MAX(frd->u.min, frs->u.min);
+		frs->u.max = RTE_MIN(frs->u.max, frd->u.max);
+	} else
+		frd->v.type = frs->v.type = BPF_ARG_UNINHABITED;
+
+	if (trd->u.min < trs->u.max) {
+		trd->u.max = RTE_MIN(trd->u.max, trs->u.max - 1);
+		trs->u.min = RTE_MAX(trs->u.min, trd->u.min + 1);
+	} else
+		trd->v.type = trs->v.type = BPF_ARG_UNINHABITED;
 }
 
 static void
 eval_jsgt_jsle(struct bpf_reg_val *trd, struct bpf_reg_val *trs,
 	struct bpf_reg_val *frd, struct bpf_reg_val *frs)
 {
-	frd->s.max = RTE_MIN(frd->s.max, frs->s.max);
-	frs->s.min = RTE_MAX(frs->s.min, frd->s.min);
-	trd->s.min = RTE_MAX(trd->s.min, trs->s.min + 1);
-	trs->s.max = RTE_MIN(trs->s.max, trd->s.max - 1);
+	if (frd->s.min <= frs->s.max) {
+		frd->s.max = RTE_MIN(frd->s.max, frs->s.max);
+		frs->s.min = RTE_MAX(frs->s.min, frd->s.min);
+	} else
+		frd->v.type = frs->v.type = BPF_ARG_UNINHABITED;
+
+	if (trs->s.min < trd->s.max) {
+		trd->s.min = RTE_MAX(trd->s.min, trs->s.min + 1);
+		trs->s.max = RTE_MIN(trs->s.max, trd->s.max - 1);
+	} else
+		trd->v.type = trs->v.type = BPF_ARG_UNINHABITED;
 }
 
 static void
 eval_jslt_jsge(struct bpf_reg_val *trd, struct bpf_reg_val *trs,
 	struct bpf_reg_val *frd, struct bpf_reg_val *frs)
 {
-	frd->s.min = RTE_MAX(frd->s.min, frs->s.min);
-	frs->s.max = RTE_MIN(frs->s.max, frd->s.max);
-	trd->s.max = RTE_MIN(trd->s.max, trs->s.max - 1);
-	trs->s.min = RTE_MAX(trs->s.min, trd->s.min + 1);
+	if (frs->s.min <= frd->s.max) {
+		frd->s.min = RTE_MAX(frd->s.min, frs->s.min);
+		frs->s.max = RTE_MIN(frs->s.max, frd->s.max);
+	} else
+		frd->v.type = frs->v.type = BPF_ARG_UNINHABITED;
+
+	if (trd->s.min < trs->s.max) {
+		trd->s.max = RTE_MIN(trd->s.max, trs->s.max - 1);
+		trs->s.min = RTE_MAX(trs->s.min, trd->s.min + 1);
+	} else
+		trd->v.type = trs->v.type = BPF_ARG_UNINHABITED;
 }
 
 static const char *
@@ -1609,6 +1649,14 @@ eval_jcc(struct bpf_verifier *bvf, const struct ebpf_insn *ins)
 	else if (op == EBPF_JSGE)
 		eval_jslt_jsge(frd, frs, trd, trs);
 
+	if (trd->v.type == BPF_ARG_UNINHABITED ||
+			trs->v.type == BPF_ARG_UNINHABITED)
+		tst->unreachable = true;
+
+	if (frd->v.type == BPF_ARG_UNINHABITED ||
+			frs->v.type == BPF_ARG_UNINHABITED)
+		fst->unreachable = true;
+
 	return NULL;
 }
 
@@ -2349,7 +2397,7 @@ set_edge_type(struct bpf_verifier *bvf, struct inst_node *node,
  * Depth-First Search (DFS) through previously constructed
  * Control Flow Graph (CFG).
  * Information collected at this path would be used later
- * to determine is there any loops, and/or unreachable instructions.
+ * to determine is there any loops, and/or statically unreachable instructions.
  * PREREQUISITE: there is at least one node.
  */
 static void
@@ -2397,7 +2445,7 @@ dfs(struct bpf_verifier *bvf)
 }
 
 /*
- * report unreachable instructions.
+ * report statically unreachable instructions.
  */
 static void
 log_unreachable(const struct bpf_verifier *bvf)
@@ -2970,13 +3018,21 @@ evaluate(struct bpf_verifier *bvf)
 				stats.nb_restore++;
 			}
 
+			if (bvf->evst->unreachable) {
+				rc = __rte_bpf_validate_debug_evaluate_step(
+					debug, get_node_idx(bvf, next),
+					RTE_BPF_VALIDATE_DEBUG_EVENT_BRANCH_UNREACHABLE);
+				if (rc < 0)
+					break;
+
+				next = NULL;
 			/*
 			 * for jcc targets: check did we already evaluated
 			 * that path and can it's evaluation be skipped that
 			 * time.
 			 */
-			if (node->nb_edge > 1 && prune_eval_state(bvf, node,
-					next) == 0) {
+			} else if (node->nb_edge > 1 &&
+					prune_eval_state(bvf, node, next) == 0) {
 				rc = __rte_bpf_validate_debug_evaluate_step(
 					debug, get_node_idx(bvf, next),
 					RTE_BPF_VALIDATE_DEBUG_EVENT_BRANCH_PRUNE);
diff --git a/lib/bpf/rte_bpf_validate_debug.h b/lib/bpf/rte_bpf_validate_debug.h
index 89bf587f0211..f30fa926f10a 100644
--- a/lib/bpf/rte_bpf_validate_debug.h
+++ b/lib/bpf/rte_bpf_validate_debug.h
@@ -49,6 +49,8 @@ enum rte_bpf_validate_debug_event {
 	RTE_BPF_VALIDATE_DEBUG_EVENT_BRANCH_PRUNE,
 	/* End of branch verification, after the last verified instruction. */
 	RTE_BPF_VALIDATE_DEBUG_EVENT_BRANCH_RETURN,
+	/* Pruning branch as dynamically unreachable. */
+	RTE_BPF_VALIDATE_DEBUG_EVENT_BRANCH_UNREACHABLE,
 	/* Number of valid event values. */
 	RTE_BPF_VALIDATE_DEBUG_EVENT_END,
 };
-- 
2.43.0


^ permalink raw reply related

* [PATCH v3 18/25] bpf/validate: fix BPF_AND min calculations
From: Marat Khalili @ 2026-06-12 10:47 UTC (permalink / raw)
  To: Konstantin Ananyev; +Cc: dev, stable, Claudia Cauli
In-Reply-To: <20260612104743.6465-1-marat.khalili@huawei.com>

Function `eval_and` calculated both signed (if positive) and unsigned
minimum values as bitwise AND between corresponding minimums, which is
incorrect since intermediate values can have zeroes in bits where
minimum values don't.

E.g. consider the following program with the current validation code:

    Tested program:
        0:  mov r0, #0x0
        1:  ldxdw r2, [r1 + 0]
        2:  jlt r2, #0x6, L8
        3:  jgt r2, #0x8, L8
        4:  jslt r2, #0x6, L8
        5:  jsgt r2, #0x8, L8
        6:  and r2, #0x5  ; tested instruction
        7:  mov r0, #0x1
        8:  exit
    Pre-state:
       r2:  6..8
    Post-state:
       r2:  4..7

After the tested instruction validator considers r2 to be equal or
greater than 4, however if 8 was loaded on step 1 it is possible for it
to be zero (0x8 & 0x5 == 0).

Use zero as a new safe lower bound for both signed (if positive) and
unsigned minimum. Add test.

Fixes: 8021917293d0 ("bpf: add extra validation for input BPF program")
Cc: stable@dpdk.org

Reported-by: Claudia Cauli <claudiacauli@gmail.com>
Signed-off-by: Marat Khalili <marat.khalili@huawei.com>
Acked-by: Konstantin Ananyev <konstantin.ananyev@huawei.com>
---
Depends-on: series-38434 ("bpf: introduce extensible load API")
 app/test/test_bpf_validate.c | 17 +++++++++++++++++
 lib/bpf/bpf_validate.c       |  4 ++--
 2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/app/test/test_bpf_validate.c b/app/test/test_bpf_validate.c
index f80dee24a677..5d26299ba65d 100644
--- a/app/test/test_bpf_validate.c
+++ b/app/test/test_bpf_validate.c
@@ -1384,6 +1384,23 @@ test_alu64_add_x_scalar_scalar(void)
 REGISTER_FAST_TEST(bpf_validate_alu64_add_x_scalar_scalar_autotest, NOHUGE_OK, ASAN_OK,
 	test_alu64_add_x_scalar_scalar);
 
+/* 64-bit bitwise AND between a scalar range and immediate. */
+static int
+test_alu64_and_k(void)
+{
+	return verify_instruction((struct verify_instruction_param){
+		.tested_instruction = {
+			.code = (EBPF_ALU64 | BPF_AND | BPF_K),
+			.imm = 5,
+		},
+		.pre.dst = make_signed_domain(6, 8),
+		.post.dst = make_signed_domain(0, 7),
+	});
+}
+
+REGISTER_FAST_TEST(bpf_validate_alu64_and_k_autotest, NOHUGE_OK, ASAN_OK,
+	test_alu64_and_k);
+
 /* 64-bit division and modulo of UINT64_MAX*2/3. */
 static int
 test_alu64_div_mod_big_constant(void)
diff --git a/lib/bpf/bpf_validate.c b/lib/bpf/bpf_validate.c
index af084e36c8d0..d4d8ec4251f1 100644
--- a/lib/bpf/bpf_validate.c
+++ b/lib/bpf/bpf_validate.c
@@ -848,7 +848,7 @@ eval_and(struct bpf_reg_val *rd, const struct bpf_reg_val *rs, size_t opsz,
 		rd->u.max &= rs->u.max;
 	} else {
 		rd->u.max = eval_uand_max(rd->u.max, rs->u.max, opsz);
-		rd->u.min &= rs->u.min;
+		rd->u.min = 0;
 	}
 
 	/* both operands are constants */
@@ -859,7 +859,7 @@ eval_and(struct bpf_reg_val *rd, const struct bpf_reg_val *rs, size_t opsz,
 	} else if (rd->s.min >= 0 || rs->s.min >= 0) {
 		rd->s.max = eval_uand_max(rd->s.max & (msk >> 1),
 			rs->s.max & (msk >> 1), opsz);
-		rd->s.min &= rs->s.min;
+		rd->s.min = 0;
 	} else
 		eval_smax_bound(rd, msk);
 }
-- 
2.43.0


^ permalink raw reply related

* [PATCH v3 19/25] bpf/validate: fix BPF_LSH shift-out-of-bounds UB
From: Marat Khalili @ 2026-06-12 10:47 UTC (permalink / raw)
  To: Konstantin Ananyev; +Cc: dev, stable, Claudia Cauli
In-Reply-To: <20260612104743.6465-1-marat.khalili@huawei.com>

Function `eval_lsh` when validating left shift by 63 invoked macro
`RTE_LEN2MASK(0, int64_t)` which triggered shift-out-of-bounds undefined
behaviour.

E.g. consider the following program with the current validation code:

    Tested program:
        0:  mov r0, #0x0
        1:  ldxdw r2, [r1 + 0]
        2:  jlt r2, #0x3, L8
        3:  jgt r2, #0x5, L8
        4:  jslt r2, #0x3, L8
        5:  jsgt r2, #0x5, L8
        6:  lsh r2, #0x3f  ; tested instruction
        7:  mov r0, #0x1
        8:  exit
    Pre-state:
       r2:  3..5
    Post-state:
       r2:  0..UINT64_MAX

With sanitizer the following diagnostic is generated:

    lib/bpf/bpf_validate.c:785:4: runtime error: shift exponent 64 is
    too large for 64-bit type 'long unsigned int'
        #0 0x00000274d5e0 in eval_lsh lib/bpf/bpf_validate.c:785
        #1 0x00000275a2ea in eval_alu lib/bpf/bpf_validate.c:1310
        #2 0x00000276ce3d in evaluate lib/bpf/bpf_validate.c:3284

Add guard for this case, add test.

Fixes: 8021917293d0 ("bpf: add extra validation for input BPF program")
Cc: stable@dpdk.org

Reported-by: Claudia Cauli <claudiacauli@gmail.com>
Signed-off-by: Marat Khalili <marat.khalili@huawei.com>
Acked-by: Konstantin Ananyev <konstantin.ananyev@huawei.com>
---
Depends-on: series-38434 ("bpf: introduce extensible load API")
 app/test/test_bpf_validate.c | 17 +++++++++++++++++
 lib/bpf/bpf_validate.c       |  3 ++-
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/app/test/test_bpf_validate.c b/app/test/test_bpf_validate.c
index 5d26299ba65d..c3d1bdb78fbc 100644
--- a/app/test/test_bpf_validate.c
+++ b/app/test/test_bpf_validate.c
@@ -1536,6 +1536,23 @@ test_alu64_div_mod_overflow(void)
 REGISTER_FAST_TEST(bpf_validate_alu64_div_mod_overflow_autotest, NOHUGE_OK, ASAN_OK,
 	test_alu64_div_mod_overflow);
 
+/* 64-bit left shift by 63. */
+static int
+test_alu64_lsh_63(void)
+{
+	return verify_instruction((struct verify_instruction_param){
+		.tested_instruction = {
+			.code = (EBPF_ALU64 | BPF_LSH | BPF_K),
+			.imm = 63,
+		},
+		.pre.dst = make_signed_domain(3, 5),
+		.post.dst = unknown,
+	});
+}
+
+REGISTER_FAST_TEST(bpf_validate_alu64_lsh_63_autotest, NOHUGE_OK, ASAN_OK,
+	test_alu64_lsh_63);
+
 /* 64-bit multiplication of constant and immediate with overflow. */
 static int
 test_alu64_mul_k_overflow(void)
diff --git a/lib/bpf/bpf_validate.c b/lib/bpf/bpf_validate.c
index d4d8ec4251f1..4e4c0ddeb2b8 100644
--- a/lib/bpf/bpf_validate.c
+++ b/lib/bpf/bpf_validate.c
@@ -746,7 +746,8 @@ eval_lsh(struct bpf_reg_val *rd, const struct bpf_reg_val *rs, size_t opsz,
 
 	/* check that dreg values are and would remain always positive */
 	if ((uint64_t)rd->s.min >> (opsz - 1) != 0 || rd->s.max >=
-			RTE_LEN2MASK(opsz - rs->u.max - 1, int64_t))
+			(rs->u.max == opsz - 1 ? 0 :
+				 RTE_LEN2MASK(opsz - rs->u.max - 1, int64_t)))
 		eval_smax_bound(rd, msk);
 	else {
 		rd->s.max <<= rs->u.max;
-- 
2.43.0


^ permalink raw reply related

* [PATCH v3 20/25] bpf/validate: fix BPF_OR min calculations
From: Marat Khalili @ 2026-06-12 10:47 UTC (permalink / raw)
  To: Konstantin Ananyev; +Cc: dev, stable, Claudia Cauli
In-Reply-To: <20260612104743.6465-1-marat.khalili@huawei.com>

This commit fixes two different problems in signed and unsigned minimum
calculations within `eval_or`. Passing tests requires both problems to
be fixed which is why the changes are squashed in one commit.

1) Function `eval_or` calculated result signed minimum as bitwise OR
between corresponding minimums as long as any of them is non-negative,
which is incorrect since values within the range can have zeroes where
the minimums don't, including the sign bit.

E.g. consider the following program with the current validation code:

    Tested program:
        0:  mov r0, #0x0
        1:  ldxdw r2, [r1 + 0]
        2:  jlt r2, #0x5, L8
        3:  jgt r2, #0x6, L8
        4:  jslt r2, #0x5, L8
        5:  jsgt r2, #0x6, L8
        6:  or r2, #0xfffffffe  ; tested instruction
        7:  mov r0, #0x1
        8:  exit
    Pre-state:
       r2:  5..6
    Post-state:
       r2:  -1

After the tested instruction validator considers r2 to always equal -1,
however if 6 was loaded on step 1 it is possible for it to be -2:

     0x6 & 0xfffffffffffffffe == 0xfffffffffffffffe = -2

Set signed range to full if any of the operands can be negative,
otherwise use the maximum of both minimums as a new signed minimum
following the idea that result of bitwise OR cannot be smaller than its
operands. Add test.

2) Function `eval_or` calculated result unsigned minimum as bitwise OR
between corresponding minimums, which is incorrect since values within
the range can have zeroes the minimums don't.

E.g. consider the following program with the current validation code:

    Tested program:
        0:  mov r0, #0x0
        1:  ldxdw r2, [r1 + 0]
        2:  jlt r2, #0x5, L8
        3:  jgt r2, #0x6, L8
        4:  jslt r2, #0x5, L8
        5:  jsgt r2, #0x6, L8
        6:  or r2, #0x2  ; tested instruction
        7:  mov r0, #0x1
        8:  exit
    Pre-state:
       r2:  5..6
    Post-state:
       r2:  7

After the tested instruction validator considers r2 to always equal 7,
however if 6 was loaded on step 1 it is possible for it to be 6:

    0x6 & 0x2 == 0x6

Use the maximum of both minimums as a new unsigned minimum following the
idea that result of bitwise OR cannot be smaller than its operands. Add
test.

Fixes: 8021917293d0 ("bpf: add extra validation for input BPF program")
Cc: stable@dpdk.org

Reported-by: Claudia Cauli <claudiacauli@gmail.com>
Signed-off-by: Marat Khalili <marat.khalili@huawei.com>
Acked-by: Konstantin Ananyev <konstantin.ananyev@huawei.com>
---
Depends-on: series-38434 ("bpf: introduce extensible load API")
 app/test/test_bpf_validate.c | 34 ++++++++++++++++++++++++++++++++++
 lib/bpf/bpf_validate.c       |  6 +++---
 2 files changed, 37 insertions(+), 3 deletions(-)

diff --git a/app/test/test_bpf_validate.c b/app/test/test_bpf_validate.c
index c3d1bdb78fbc..7cf9e404b697 100644
--- a/app/test/test_bpf_validate.c
+++ b/app/test/test_bpf_validate.c
@@ -1713,6 +1713,40 @@ test_alu64_neg_zero_last(void)
 REGISTER_FAST_TEST(bpf_validate_alu64_neg_zero_last_autotest, NOHUGE_OK, ASAN_OK,
 	test_alu64_neg_zero_last);
 
+/* 64-bit bitwise OR between a positive scalar range and negative immediate. */
+static int
+test_alu64_or_k_negative(void)
+{
+	return verify_instruction((struct verify_instruction_param){
+		.tested_instruction = {
+			.code = (EBPF_ALU64 | BPF_OR | BPF_K),
+			.imm = -2,
+		},
+		.pre.dst = make_signed_domain(5, 6),
+		.post.dst = make_signed_domain(-2, -1),
+	});
+}
+
+REGISTER_FAST_TEST(bpf_validate_alu64_or_k_negative_autotest, NOHUGE_OK, ASAN_OK,
+	test_alu64_or_k_negative);
+
+/* 64-bit bitwise OR between a positive scalar range and positive immediate. */
+static int
+test_alu64_or_k_positive(void)
+{
+	return verify_instruction((struct verify_instruction_param){
+		.tested_instruction = {
+			.code = (EBPF_ALU64 | BPF_OR | BPF_K),
+			.imm = 2,
+		},
+		.pre.dst = make_signed_domain(5, 6),
+		.post.dst = make_signed_domain(5, 7),
+	});
+}
+
+REGISTER_FAST_TEST(bpf_validate_alu64_or_k_positive_autotest, NOHUGE_OK, ASAN_OK,
+	test_alu64_or_k_positive);
+
 /* Jump if greater than immediate. */
 static int
 test_jmp64_jeq_k(void)
diff --git a/lib/bpf/bpf_validate.c b/lib/bpf/bpf_validate.c
index 4e4c0ddeb2b8..abb39cfd328d 100644
--- a/lib/bpf/bpf_validate.c
+++ b/lib/bpf/bpf_validate.c
@@ -875,7 +875,7 @@ eval_or(struct bpf_reg_val *rd, const struct bpf_reg_val *rs, size_t opsz,
 		rd->u.max |= rs->u.max;
 	} else {
 		rd->u.max = eval_uor_max(rd->u.max, rs->u.max, opsz);
-		rd->u.min |= rs->u.min;
+		rd->u.min = RTE_MAX(rd->u.min, rs->u.min);
 	}
 
 	/* both operands are constants */
@@ -884,9 +884,9 @@ eval_or(struct bpf_reg_val *rd, const struct bpf_reg_val *rs, size_t opsz,
 		rd->s.max |= rs->s.max;
 
 	/* both operands are non-negative */
-	} else if (rd->s.min >= 0 || rs->s.min >= 0) {
+	} else if (rd->s.min >= 0 && rs->s.min >= 0) {
 		rd->s.max = eval_uor_max(rd->s.max, rs->s.max, opsz);
-		rd->s.min |= rs->s.min;
+		rd->s.min = RTE_MAX(rd->s.min, rs->s.min);
 	} else
 		eval_smax_bound(rd, msk);
 }
-- 
2.43.0


^ permalink raw reply related

* [PATCH v3 21/25] bpf/validate: fix BPF_SUB signed max zero case
From: Marat Khalili @ 2026-06-12 10:47 UTC (permalink / raw)
  To: Konstantin Ananyev; +Cc: dev, stable, Claudia Cauli
In-Reply-To: <20260612104743.6465-1-marat.khalili@huawei.com>

Function `eval_sub` used source register signed minimum to detect
overflow of the difference (operation result) signed minimum, and source
register signed maximum to detect overflow of the difference signed
maximum. However in the actual formula for difference source register
bounds are swapped (correctly, since we subtract it), so in overflow
detection we should also have swapped them. It caused false negatives in
certain cases.

E.g. consider the following program with the current validation code:

    Tested program:
        0:  mov r0, #0x0
        1:  ldxdw r2, [r1 + 0]
        2:  jsgt r2, #0x0, L7
        3:  ldxdw r3, [r1 + 8]
        4:  jsgt r3, #0x0, L7
        5:  sub r2, r3  ; tested instruction
        6:  mov r0, #0x1
        7:  exit
    Pre-state:
       r2:  INT64_MIN..0
       r3:  INT64_MIN..0
    Post-state:
       r2:  INT64_MIN

Validator ignores overflow of signed minimum and considers result to
always equal INT64_MIN. However, if -1 was loaded on step 1 and -2 was
loaded on step 3 it is possible for the difference to equal 1.

Swap source register signed minimum and maximum in the overflow
condition to match the new range formula, add test.

Fixes: 8021917293d0 ("bpf: add extra validation for input BPF program")
Cc: stable@dpdk.org

Reported-by: Claudia Cauli <claudiacauli@gmail.com>
Signed-off-by: Marat Khalili <marat.khalili@huawei.com>
Acked-by: Konstantin Ananyev <konstantin.ananyev@huawei.com>
---
Depends-on: series-38434 ("bpf: introduce extensible load API")
 app/test/test_bpf_validate.c | 17 +++++++++++++++++
 lib/bpf/bpf_validate.c       |  4 ++--
 2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/app/test/test_bpf_validate.c b/app/test/test_bpf_validate.c
index 7cf9e404b697..205373a4f86b 100644
--- a/app/test/test_bpf_validate.c
+++ b/app/test/test_bpf_validate.c
@@ -1747,6 +1747,23 @@ test_alu64_or_k_positive(void)
 REGISTER_FAST_TEST(bpf_validate_alu64_or_k_positive_autotest, NOHUGE_OK, ASAN_OK,
 	test_alu64_or_k_positive);
 
+/* 64-bit difference between two negative ranges.. */
+static int
+test_alu64_sub_x_src_signed_max_zero(void)
+{
+	return verify_instruction((struct verify_instruction_param){
+		.tested_instruction = {
+			.code = (EBPF_ALU64 | BPF_SUB | BPF_X),
+		},
+		.pre.dst = make_signed_domain(INT64_MIN, 0),
+		.pre.src = make_signed_domain(INT64_MIN, 0),
+		.post.dst = unknown,
+	});
+}
+
+REGISTER_FAST_TEST(bpf_validate_alu64_sub_x_src_signed_max_zero_autotest, NOHUGE_OK, ASAN_OK,
+	test_alu64_sub_x_src_signed_max_zero);
+
 /* Jump if greater than immediate. */
 static int
 test_jmp64_jeq_k(void)
diff --git a/lib/bpf/bpf_validate.c b/lib/bpf/bpf_validate.c
index abb39cfd328d..131a5468dbc4 100644
--- a/lib/bpf/bpf_validate.c
+++ b/lib/bpf/bpf_validate.c
@@ -716,9 +716,9 @@ eval_sub(struct bpf_reg_val *rd, const struct bpf_reg_val *rs, uint64_t msk)
 		eval_umax_bound(&rv, msk);
 
 	if ((rd->s.min != rd->s.max || rs->s.min != rs->s.max) &&
-			(((rs->s.min < 0 && rv.s.min < rd->s.min) ||
+			(((rs->s.max < 0 && rv.s.min < rd->s.min) ||
 			rv.s.min > rd->s.min) ||
-			((rs->s.max < 0 && rv.s.max < rd->s.max) ||
+			((rs->s.min < 0 && rv.s.max < rd->s.max) ||
 			rv.s.max > rd->s.max)))
 		eval_smax_bound(&rv, msk);
 
-- 
2.43.0


^ permalink raw reply related

* [PATCH v3 22/25] bpf/validate: fix BPF_XOR signed min calculation
From: Marat Khalili @ 2026-06-12 10:47 UTC (permalink / raw)
  To: Konstantin Ananyev; +Cc: dev, stable, Claudia Cauli
In-Reply-To: <20260612104743.6465-1-marat.khalili@huawei.com>

Function `eval_xor` calculated signed minimum using essentially unsigned
algorithm as long as any of the operands have non-negative range, which
is incorrect since it ignores any negative numbers that may have the
sign or any other bits set.

E.g. consider the following program with the current validation code:

    Tested program:
        0:  mov r0, #0x0
        1:  ldxdw r2, [r1 + 0]
        2:  jsgt r2, #0x0, L5
        3:  xor r2, #0x0  ; tested instruction
        4:  mov r0, #0x1
        5:  exit
    Pre-state:
       r2:  INT64_MIN..0
    Post-state:
       r2:  0

After the tested instruction validator considers r2 to equal 0, however
if -1 was loaded on step 1 it is possible for it to be -1.

Set signed range to full if any of the operands can be negative,
otherwise (if both operands are non-negative) use same algorithm as for
unsigned numbers. Add test.

Fixes: 8021917293d0 ("bpf: add extra validation for input BPF program")
Cc: stable@dpdk.org

Reported-by: Claudia Cauli <claudiacauli@gmail.com>
Signed-off-by: Marat Khalili <marat.khalili@huawei.com>
Acked-by: Konstantin Ananyev <konstantin.ananyev@huawei.com>
---
Depends-on: series-38434 ("bpf: introduce extensible load API")
 app/test/test_bpf_validate.c | 17 +++++++++++++++++
 lib/bpf/bpf_validate.c       |  2 +-
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/app/test/test_bpf_validate.c b/app/test/test_bpf_validate.c
index 205373a4f86b..a06d3254d6ba 100644
--- a/app/test/test_bpf_validate.c
+++ b/app/test/test_bpf_validate.c
@@ -1764,6 +1764,23 @@ test_alu64_sub_x_src_signed_max_zero(void)
 REGISTER_FAST_TEST(bpf_validate_alu64_sub_x_src_signed_max_zero_autotest, NOHUGE_OK, ASAN_OK,
 	test_alu64_sub_x_src_signed_max_zero);
 
+/* 64-bit bitwise XOR between a negative scalar range and zero immediate. */
+static int
+test_alu64_xor_k_negative(void)
+{
+	return verify_instruction((struct verify_instruction_param){
+		.tested_instruction = {
+			.code = (EBPF_ALU64 | BPF_XOR | BPF_K),
+			.imm = 0,
+		},
+		.pre.dst = make_signed_domain(INT64_MIN, 0),
+		.post.dst = unknown,
+	});
+}
+
+REGISTER_FAST_TEST(bpf_validate_alu64_xor_k_negative_autotest, NOHUGE_OK, ASAN_OK,
+	test_alu64_xor_k_negative);
+
 /* Jump if greater than immediate. */
 static int
 test_jmp64_jeq_k(void)
diff --git a/lib/bpf/bpf_validate.c b/lib/bpf/bpf_validate.c
index 131a5468dbc4..03c590c75377 100644
--- a/lib/bpf/bpf_validate.c
+++ b/lib/bpf/bpf_validate.c
@@ -910,7 +910,7 @@ eval_xor(struct bpf_reg_val *rd, const struct bpf_reg_val *rs, size_t opsz,
 		rd->s.max ^= rs->s.max;
 
 	/* both operands are non-negative */
-	} else if (rd->s.min >= 0 || rs->s.min >= 0) {
+	} else if (rd->s.min >= 0 && rs->s.min >= 0) {
 		rd->s.max = eval_uor_max(rd->s.max, rs->s.max, opsz);
 		rd->s.min = 0;
 	} else
-- 
2.43.0


^ permalink raw reply related

* [PATCH v3 23/25] bpf/validate: prevent overflow when building graph
From: Marat Khalili @ 2026-06-12 10:47 UTC (permalink / raw)
  To: Konstantin Ananyev; +Cc: dev, stable, Claudia Cauli
In-Reply-To: <20260612104743.6465-1-marat.khalili@huawei.com>

Function `evst_pool_init` for malicious or corrupt BPF program with
number of conditional jumps exceeding a third of UINT32_MAX could cause
arithmetic and buffer overflows when working with the program graph.

Fix the issue by limiting maximum number of conditional jumps supported
by UINT32_MAX / 4, or more than 1 billion.

Fixes: 8021917293d0 ("bpf: add extra validation for input BPF program")
Cc: stable@dpdk.org

Reported-by: Claudia Cauli <claudiacauli@gmail.com>
Signed-off-by: Marat Khalili <marat.khalili@huawei.com>
Acked-by: Konstantin Ananyev <konstantin.ananyev@huawei.com>
---
Depends-on: series-38434 ("bpf: introduce extensible load API")
 lib/bpf/bpf_validate.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/lib/bpf/bpf_validate.c b/lib/bpf/bpf_validate.c
index 03c590c75377..f9960088a285 100644
--- a/lib/bpf/bpf_validate.c
+++ b/lib/bpf/bpf_validate.c
@@ -2662,6 +2662,10 @@ evst_pool_init(struct bpf_verifier *bvf)
 {
 	uint32_t k, n;
 
+	if (bvf->nb_jcc_nodes > UINT32_MAX / 4)
+		/* Calculations that follow may overflow. */
+		return -E2BIG;
+
 	/*
 	 * We need nb_jcc_nodes + 1 for save_cur/restore_cur
 	 * remaining ones will be used for state tracking/pruning.
-- 
2.43.0


^ permalink raw reply related

* [PATCH v3 24/25] doc: add release notes for BPF validation fixes
From: Marat Khalili @ 2026-06-12 10:47 UTC (permalink / raw)
  Cc: dev, Konstantin Ananyev
In-Reply-To: <20260612104743.6465-1-marat.khalili@huawei.com>

Document the following new features and fixes:
* Added BPF validation debugger API (rte_bpf_validate_debug_*).
* Hardened BPF validator with numerous bug fixes and UB preventions.

Signed-off-by: Marat Khalili <marat.khalili@huawei.com>
Acked-by: Konstantin Ananyev <konstantin.ananyev@huawei.com>
---
Depends-on: series-38434 ("bpf: introduce extensible load API")
 doc/guides/rel_notes/release_26_07.rst | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/doc/guides/rel_notes/release_26_07.rst b/doc/guides/rel_notes/release_26_07.rst
index 3a151c8e83bb..d98e73b8340f 100644
--- a/doc/guides/rel_notes/release_26_07.rst
+++ b/doc/guides/rel_notes/release_26_07.rst
@@ -175,6 +175,22 @@ New Features
   ``rte_bpf_eth_tx_install`` for installing already loaded BPF programs as
   port callbacks (as opposed to loading them directly from ELF files).
 
+* **Hardened BPF validator.**
+
+  Fixed numerous bugs in the BPF validator's abstract interpretation logic,
+  including incorrect bounds tracking for jumps and arithmetic operations, as
+  well as fixing several instances of undefined behavior (UB) when verifying
+  malicious or corrupt programs.
+
+* **Added BPF validation debugger API.**
+
+  Introduced a new set of APIs (prefixed with ``rte_bpf_validate_debug_``) to
+  introspect the BPF validator. This provides a mechanism to set breakpoints or
+  catchpoints during validation and inspect the verifier's internal state
+  (such as tracked register bounds). This API is crucial primarily for writing
+  comprehensive tests for the validator, but also serves as a foundation for a
+  future interactive eBPF validation debugger.
+
 
 Removed Items
 -------------
-- 
2.43.0


^ permalink raw reply related

* [PATCH v3 25/25] doc: add BPF validate debug to programmer's guide
From: Marat Khalili @ 2026-06-12 10:47 UTC (permalink / raw)
  To: Konstantin Ananyev; +Cc: dev
In-Reply-To: <20260612104743.6465-1-marat.khalili@huawei.com>

Document the new gdb-like validation debugger API, outlining how it can
be used to set breakpoints and inspect register states during
validation.

Highlight its primary use case: writing robust tests for the eBPF
verifier using the harness in app/test/test_bpf_validate.c.

Signed-off-by: Marat Khalili <marat.khalili@huawei.com>
Acked-by: Konstantin Ananyev <konstantin.ananyev@huawei.com>
---
Depends-on: series-38434 ("bpf: introduce extensible load API")
 doc/guides/prog_guide/bpf_lib.rst | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/doc/guides/prog_guide/bpf_lib.rst b/doc/guides/prog_guide/bpf_lib.rst
index df3782508829..08ea1876875a 100644
--- a/doc/guides/prog_guide/bpf_lib.rst
+++ b/doc/guides/prog_guide/bpf_lib.rst
@@ -118,6 +118,37 @@ For example, ``(BPF_IND | BPF_W | BPF_LD)`` means:
 and ``R1-R5`` were scratched.
 
 
+Validation Debugger
+-------------------
+
+The DPDK BPF library includes a validation debugger API designed primarily for
+writing comprehensive unit tests for the eBPF verifier. It allows developers
+to introspect the abstract interpretation process step-by-step to guarantee
+that the verifier correctly models the semantics of eBPF instructions.
+
+The debugger operates using a gdb-like approach:
+
+1.  **Initialization:** Create a debug session using
+    ``rte_bpf_validate_debug_create()`` and pass it to the loader via the
+    ``debug`` field in ``struct rte_bpf_prm_ex``.
+2.  **Breakpoints and Catchpoints:** Before loading, use
+    ``rte_bpf_validate_debug_break()`` or ``rte_bpf_validate_debug_catch()``
+    to register callback functions that trigger at specific instruction indices
+    (program counters) or upon specific validation events.
+3.  **State Introspection:** Within the callbacks, the API provides functions
+    like ``rte_bpf_validate_debug_can_access()``,
+    ``rte_bpf_validate_debug_may_jump()``, and various formatting functions
+    to safely inspect the verifier's internal belief about register bounds
+    and memory states at that specific execution point.
+
+When adding a test for a new eBPF instruction or fixing a validator bug,
+developers should utilize the harness provided in
+``app/test/test_bpf_validate.c``. This harness encapsulates the debugger API,
+allowing you to define the expected abstract domains (signed and unsigned
+intervals) for registers before and after a tested instruction, generating
+the necessary eBPF bytecode and breakpoints automatically.
+
+
 Not currently supported eBPF features
 -------------------------------------
 
-- 
2.43.0


^ permalink raw reply related

* RE: [PATCH v4 02/11] bpf: introduce extensible load API
From: Marat Khalili @ 2026-06-12 10:48 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: Konstantin Ananyev, Wathsala Vithanage, dev@dpdk.org
In-Reply-To: <n9qJkNlCQDazx_P9i-5XTQ@monjalon.net>

> Unfortunately, compilation is failing on this patch:
> 
> lib/bpf/bpf_load.c:274:40: error: 'struct rte_bpf_jit' has no member named 'raw'

Sent v5 fixing the issue, apologies for sending without double-checking 
compilability of individual commits on my side.

^ permalink raw reply

* [PATCH] net/iavf: fix scalar Rx path zero-length segment
From: Ciara Loftus @ 2026-06-12 14:35 UTC (permalink / raw)
  To: dev; +Cc: Ciara Loftus, stable, Declan Doherty

When hardware CRC stripping is active, a frame whose on-wire size is an
exact multiple of the Rx buffer size can cause the NIC to fill the final
data descriptor and place the four CRC bytes into a separate trailing
descriptor. After hardware stripping, that descriptor carries zero bytes
of payload.

The existing CRC cleanup code only handles a zero-length trailing segment
when software CRC stripping is enabled. When hardware stripping is
active, the zero-length mbuf is silently chained to the reassembled
packet. Forwarding such a packet causes a zero-length Tx descriptor,
triggering a Malicious Driver Detection event on the PF and resetting
the VF.

Fix by adding logic to detect a zero-length final segment when hardware
CRC stripping is active, and freeing it.

Fixes: a2b29a7733ef ("net/avf: enable basic Rx Tx")
Fixes: b8b4c54ef9b0 ("net/iavf: support flexible Rx descriptor in normal path")
Cc: stable@dpdk.org

Signed-off-by: Declan Doherty <declan.doherty@intel.com>
Signed-off-by: Ciara Loftus <ciara.loftus@intel.com>
---
 drivers/net/intel/iavf/iavf_rxtx.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/net/intel/iavf/iavf_rxtx.c b/drivers/net/intel/iavf/iavf_rxtx.c
index a57af7faed..86ebb2618d 100644
--- a/drivers/net/intel/iavf/iavf_rxtx.c
+++ b/drivers/net/intel/iavf/iavf_rxtx.c
@@ -1716,6 +1716,14 @@ iavf_recv_scattered_pkts_flex_rxd(void *rx_queue, struct rte_mbuf **rx_pkts,
 				rxm->data_len = (uint16_t)(rx_packet_len -
 							RTE_ETHER_CRC_LEN);
 			}
+		} else if (unlikely(rx_packet_len == 0)) {
+			/*
+			 * NIC split CRC bytes into a trailing segment which is
+			 * now empty after hardware CRC stripping. Free it.
+			 */
+			rte_pktmbuf_free_seg(rxm);
+			first_seg->nb_segs--;
+			last_seg->next = NULL;
 		}
 
 		first_seg->port = rxq->port_id;
@@ -1884,6 +1892,14 @@ iavf_recv_scattered_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 			} else
 				rxm->data_len = (uint16_t)(rx_packet_len -
 							RTE_ETHER_CRC_LEN);
+		} else if (unlikely(rx_packet_len == 0)) {
+			/*
+			 * NIC split CRC bytes into a trailing segment which is
+			 * now empty after hardware CRC stripping. Free it.
+			 */
+			rte_pktmbuf_free_seg(rxm);
+			first_seg->nb_segs--;
+			last_seg->next = NULL;
 		}
 
 		first_seg->port = rxq->port_id;
-- 
2.43.0


^ permalink raw reply related

* Re: 回复:[PATCH] gpu/metax: add new driver for Metax GPU
From: Thomas Monjalon @ 2026-06-12 14:49 UTC (permalink / raw)
  To: 许玲燕; +Cc: dev, eagostini, 王冬冬
In-Reply-To: <2a306d48-351c-473e-b049-43e10e53617b.lingyan.xu@metax-tech.com>

Would it be possible to have a direct download link
not requiring any registration? At least for the library?
Do you have an english version of the website?


12/06/2026 09:19, 许玲燕:
> Hi,
> Thank you for the follow-up. Please find the clarifications below regarding the kernel dependency and availability:
> 1. Kernel Dependency
> While it is not upstreamed into the mainline Linux kernel, it is actively maintained by Metax to interface with our hardware.
> 2. Availability and Download Link
> Yes, both the proprietary user-space libraries and the corresponding kernel modules are freely available for download. You can access them via the Metax Software Download Center:
> Download Link: https://sw-download.metax-tech.com/ <https://sw-download.metax-tech.com/ >
> How to obtain the files:
> 
>  * 
> Register and log in to the portal.
> 
>  * 
> Navigate to "SDK Development Kit".
> 
>  * 
> Select your specific GPU Type.
> 
>  * 
> Choose your target OS. We currently support Linux (aarch64 & x86_64) across mainstream distributions, including but not limited to:
>  * 
> Ubuntu (18.04 - 24.04)
> 
>  * 
> RHEL / CentOS / RockyOS (8.x / 9.x)
> 
>  * 
> Domestic distros: KylinV10/V11, OpenCloudOS, TencentOS, etc.
> Best regards,
> ------------------------------------------------------------------
> 发件人:Thomas Monjalon <thomas@monjalon.net>
> 发送时间:2026年6月11日(周四) 17:17
> 收件人:"许玲燕"<lingyan.xu@metax-tech.com>
> 抄 送:dev<dev@dpdk.org>; eagostini<eagostini@nvidia.com>; "王冬冬"<dongdong.wang@metax-tech.com>
> 主 题:Re: [PATCH] gpu/metax: add new driver for Metax GPU
> 11/06/2026 09:10:
> > Both libmcruntime.so and the corresponding gdrapi libraries
> > are proprietary user-space libraries provided by Metax.
> > They are not upstreamed to the DPDK mainline repository.
> > However, please rest assured that the current patch interacts
> > with them via standard dlopen (dynamic loading) at runtime.
> > We do not link directly against their source code
> > or require them as hard build-time dependencies.
> > Therefore, this approach will not introduce any additional
> > compilation dependencies or licensing issues to the DPDK main tree.
> What about the kernel dependency?
> Are libraries and kernel module freely available for download?
> Can you provide a link?
> 






^ permalink raw reply

* Re: [PATCH 8/9] ethdev: keep fast-path ops valid after port stop
From: Thomas Monjalon @ 2026-06-12 15:00 UTC (permalink / raw)
  To: Maxime Leroy
  Cc: Morten Brørup, Hemant Agrawal, Sachin Saxena, dev, stable,
	Andrew Rybchenko, Sunil Kumar Kori
In-Reply-To: <CAHHRULUGdnTmpnAX2aONEMj3bJmN11RG2cHGJ7h3-qhjoJLsFw@mail.gmail.com>

11/06/2026 20:39, Maxime Leroy:
> Le jeu. 11 juin 2026, 18:01, Morten Brørup <mb@smartsharesystems.com> a
> écrit :
> 
> > > From: Maxime Leroy [mailto:maxime.leroys@gmail.com] On Behalf Of Maxime
> > > Leroy
> >
> > Good catch.
> > Acked-by: Morten Brørup <mb@smartsharesystems.com>
> >
> > Not related to the series, consider sending as separate patch.
> >
> Thanks for the review and Ack.
> 
> Agreed, this is a generic ethdev fix. I kept it in this series because the
> NAPI user depends on it.
> 
> The current Grout NAPI loop arms RX queue interrupts and then re-checks
> rte_eth_rx_queue_count() before blocking, to avoid sleeping when a packet
> arrived between the last empty poll and epoll_wait.
> 
> With the current ethdev reset path, rx_burst is replaced by a dummy
> callback on stop/release, but rx_queue_count becomes NULL. So if the port
> is stopped concurrently, the NAPI worker dereferences a NULL function
> pointer and
> segfaults on that recheck.
> 
> I can split it out if maintainers prefer, but then the dpaa2 NAPI series
> has a real dependency on the standalone ethdev fix.

You can keep this patch in the series AND
send it separately for a quick merge.



^ permalink raw reply

* Re: [PATCH] app/testpmd: add padding mode to txonly engine
From: Stephen Hemminger @ 2026-06-12 15:20 UTC (permalink / raw)
  To: Xingui Yang
  Cc: dev, david.marchand, aman.deep.singh, fengchengwen, yangshuaisong,
	lihuisong, liuyonglong, kangfenglong
In-Reply-To: <20260612073715.2739007-1-yangxingui@huawei.com>

On Fri, 12 Jun 2026 15:37:15 +0800
Xingui Yang <yangxingui@huawei.com> wrote:

> Add a new padding mode to the txonly forwarding engine, which allows
> sending packets with configurable small sizes without standard L2/L3
> headers. This is useful for testing NIC padding logic.
> 
> When padding mode is enabled via --tx-pkt-pad-mode flag:
> - l2_len and l3_len are set to 0 instead of standard header lengths
> - Packet data is filled with a static pattern instead of
>   Ethernet/IP/UDP headers
> - Minimum packet length validation is bypassed to allow small
>   packet sizes (e.g., set txpkts 14)
> 
> Signed-off-by: Xingui Yang <yangxingui@huawei.com>
> Signed-off-by: Huisong Li <lihuisong@huawei.com>
> ---

Why add yet another setting to already bloated testpmd command?
Instead I would suggest allowing user to specify any length from
14 up to UINT32_MAX.  The code to format packet would need to
handle it there.

^ permalink raw reply

* Re: [PATCH] net/iavf: fix scalar Rx path zero-length segment
From: Bruce Richardson @ 2026-06-12 15:42 UTC (permalink / raw)
  To: Ciara Loftus; +Cc: dev, stable, Declan Doherty
In-Reply-To: <20260612143531.2265914-1-ciara.loftus@intel.com>

On Fri, Jun 12, 2026 at 02:35:31PM +0000, Ciara Loftus wrote:
> When hardware CRC stripping is active, a frame whose on-wire size is an
> exact multiple of the Rx buffer size can cause the NIC to fill the final
> data descriptor and place the four CRC bytes into a separate trailing
> descriptor. After hardware stripping, that descriptor carries zero bytes
> of payload.
> 
> The existing CRC cleanup code only handles a zero-length trailing segment
> when software CRC stripping is enabled. When hardware stripping is
> active, the zero-length mbuf is silently chained to the reassembled
> packet. Forwarding such a packet causes a zero-length Tx descriptor,
> triggering a Malicious Driver Detection event on the PF and resetting
> the VF.
> 
> Fix by adding logic to detect a zero-length final segment when hardware
> CRC stripping is active, and freeing it.
> 
> Fixes: a2b29a7733ef ("net/avf: enable basic Rx Tx")
> Fixes: b8b4c54ef9b0 ("net/iavf: support flexible Rx descriptor in normal path")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Declan Doherty <declan.doherty@intel.com>
> Signed-off-by: Ciara Loftus <ciara.loftus@intel.com>
> ---
>  drivers/net/intel/iavf/iavf_rxtx.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/drivers/net/intel/iavf/iavf_rxtx.c b/drivers/net/intel/iavf/iavf_rxtx.c
> index a57af7faed..86ebb2618d 100644
> --- a/drivers/net/intel/iavf/iavf_rxtx.c
> +++ b/drivers/net/intel/iavf/iavf_rxtx.c
> @@ -1716,6 +1716,14 @@ iavf_recv_scattered_pkts_flex_rxd(void *rx_queue, struct rte_mbuf **rx_pkts,
>  				rxm->data_len = (uint16_t)(rx_packet_len -
>  							RTE_ETHER_CRC_LEN);
>  			}
> +		} else if (unlikely(rx_packet_len == 0)) {
> +			/*
> +			 * NIC split CRC bytes into a trailing segment which is
> +			 * now empty after hardware CRC stripping. Free it.
> +			 */
> +			rte_pktmbuf_free_seg(rxm);
> +			first_seg->nb_segs--;
> +			last_seg->next = NULL;
>  		}
>  

The vector paths also handle scattered packets (via reassembly). Do they
need a fix for this? What about the other drivers that work on the PF, such
as ice/i40e?

/Bruce

>  		first_seg->port = rxq->port_id;
> @@ -1884,6 +1892,14 @@ iavf_recv_scattered_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
>  			} else
>  				rxm->data_len = (uint16_t)(rx_packet_len -
>  							RTE_ETHER_CRC_LEN);
> +		} else if (unlikely(rx_packet_len == 0)) {
> +			/*
> +			 * NIC split CRC bytes into a trailing segment which is
> +			 * now empty after hardware CRC stripping. Free it.
> +			 */
> +			rte_pktmbuf_free_seg(rxm);
> +			first_seg->nb_segs--;
> +			last_seg->next = NULL;
>  		}
>  
>  		first_seg->port = rxq->port_id;
> -- 
> 2.43.0
> 

^ permalink raw reply

* Re: [PATCH v5 00/11] bpf: introduce extensible load API
From: Stephen Hemminger @ 2026-06-12 16:06 UTC (permalink / raw)
  To: Marat Khalili; +Cc: dev
In-Reply-To: <20260612084219.38399-1-marat.khalili@huawei.com>

On Fri, 12 Jun 2026 09:42:07 +0100
Marat Khalili <marat.khalili@huawei.com> wrote:

> This patchset introduces an extensible load API for the BPF library in
> DPDK, addressing current limitations regarding ABI stability and feature
> constraints.
> 
> Currently, `rte_bpf_load` relies on a fixed `struct rte_bpf_prm`, which
> makes it difficult to add new loading options or parameters without
> breaking the ABI.
> 
> To resolve these issues, this series introduces `rte_bpf_load_ex` taking
> `struct rte_bpf_prm_ex`. The new parameter structure includes a `sz`
> field for backward compatibility, allowing future extensions.
> 
> Taking advantage of the new extensible API, this patchset also adds
> several new features:
> * Support for loading and executing BPF programs with up to 5 arguments.
> * Support for loading classic BPF (cBPF) directly.
> * Support for loading ELF files directly from memory buffers.
> * New API functions (`rte_bpf_eth_rx_install` and `rte_bpf_eth_tx_install`)
>   to install an already loaded BPF program as a port callback, decoupling
>   the loading phase from the installation phase.
> 
> 
> v5:
> * Fixed compilation between commits broken in v4 while addressing AI
>   comments.
> * Rebased on fresh main, addressing conflicts in release notes.
> 
> v4:
> * Restored missing NULL checks in wrapper functions `rte_bpf_load` and
>   `rte_bpf_elf_load`.
> * Fixed the burst execution functions (`rte_bpf_exec_burst*`) to return
>   `0` and set `rte_errno = EINVAL` on failure, preventing `-EINVAL`
>   being reinterpreted as a large `uint32_t` value. Initialized `rc`
>   properly in scalar execution wrappers for this case.
> * Swapped the Doxygen comments in `rte_bpf_ethdev.h` for RX and TX functions.
> * Added diagnostic dump on failure path in `test_bpf_filter`.
> * Fixed memory leak of the BPF handle in `bpf_rx_test` upon install failure.
> * Added tests for NULL parameter rejection, mismatched execution arguments,
>   unsupported execution flags, and the libpcap-less `rte_bpf_convert` stub.
> 
> v3:
> * Appended Acked-by tags to all individual commits to align with
>   patchwork requirements.
> 
> v2:
> * Fixed a potential segmentation fault in `exec_vm_burst_ex` by deferring
>   the dereference of `ctx[i].arg` until it is confirmed that `nb_prog_arg > 0`.
> * Clarified documentation and code comments for `RTE_BPF_EXEC_FLAG_JIT`
>   requirements and fast-path expectations.
> 
> 
> Marat Khalili (11):
>   bpf: make logging prefixes more consistent
>   bpf: introduce extensible load API
>   bpf: support up to 5 arguments
>   bpf: add cBPF origin to rte_bpf_load_ex
>   bpf: support rte_bpf_prm_ex with port callbacks
>   bpf: support loading ELF files from memory
>   test/bpf: test loading cBPF directly
>   test/bpf: test loading ELF file from memory
>   doc: add release notes for new extensible BPF API
>   doc: add load API to BPF programmer's guide
>   test/bpf: add tests for error handling contracts
> 
>  app/test/test_bpf.c                    | 455 +++++++++++++++++--------
>  doc/guides/prog_guide/bpf_lib.rst      |  75 +++-
>  doc/guides/rel_notes/release_26_07.rst |  20 ++
>  lib/bpf/bpf.c                          |  32 +-
>  lib/bpf/bpf_convert.c                  |  99 +++++-
>  lib/bpf/bpf_exec.c                     | 134 +++++++-
>  lib/bpf/bpf_impl.h                     |  53 ++-
>  lib/bpf/bpf_jit_arm64.c                |  18 +-
>  lib/bpf/bpf_jit_x86.c                  |  10 +-
>  lib/bpf/bpf_load.c                     | 213 ++++++++++--
>  lib/bpf/bpf_load_elf.c                 | 189 ++++++----
>  lib/bpf/bpf_pkt.c                      |  65 +++-
>  lib/bpf/bpf_stub.c                     |  46 ---
>  lib/bpf/bpf_validate.c                 |  94 +++--
>  lib/bpf/meson.build                    |  15 +-
>  lib/bpf/rte_bpf.h                      | 199 ++++++++++-
>  lib/bpf/rte_bpf_ethdev.h               |  54 +++
>  17 files changed, 1400 insertions(+), 371 deletions(-)
>  delete mode 100644 lib/bpf/bpf_stub.c
> 

Detailed AI review found some minor things:

02/11: comment grammar nits — "Any features that not known to
the application" -> "that are not known" (twice); "the size of
same struct" -> "the size of the same struct".

05/11: Doxygen for rte_bpf_eth_rx_install and
rte_bpf_eth_tx_install references rte_bpf_eth_unload, which does
not exist. Use rte_bpf_eth_rx_unload and rte_bpf_eth_tx_unload
respectively.

07/11: stray space before comma in the test_bpf_filter log
string ("for \"%s\" ,").

^ permalink raw reply

* Re: [PATCH v2] app/testpmd: add padding mode to txonly engine
From: Stephen Hemminger @ 2026-06-12 16:13 UTC (permalink / raw)
  To: Xingui Yang
  Cc: dev, david.marchand, aman.deep.singh, fengchengwen, yangshuaisong,
	lihuisong, liuyonglong, kangfenglong
In-Reply-To: <20260612091217.2899755-1-yangxingui@huawei.com>

On Fri, 12 Jun 2026 17:12:17 +0800
Xingui Yang <yangxingui@huawei.com> wrote:

> Add a new padding mode to the txonly forwarding engine, which allows
> sending packets with configurable small sizes without standard L2/L3
> headers. This is useful for testing NIC padding logic.
> 
> When padding mode is enabled via --tx-pkt-pad-mode flag:
> - l2_len and l3_len are set to 0 instead of standard header lengths
> - Packet data is filled with a static pattern instead of
>   Ethernet/IP/UDP headers
> - Minimum packet length validation is bypassed to allow small
>   packet sizes (e.g., set txpkts 14)
> 
> Signed-off-by: Xingui Yang <yangxingui@huawei.com>
> Signed-off-by: Huisong Li <lihuisong@huawei.com>
> ---
> v2: Fix compilation exception of unterminated-string-initialization
> ---

What about something like this (*not tested*) patch.

From 84ff35849f9881a93eed65ccd43a5cd1197cecb8 Mon Sep 17 00:00:00 2001
From: Stephen Hemminger <stephen@networkplumber.org>
Date: Fri, 12 Jun 2026 09:10:17 -0700
Subject: [PATCH] testpnd: allow configuring runt frames

Allow setting transmit size to be a small value which has
ethernet header but no IP or UDP header.
---
 app/test-pmd/config.c                       | 12 +++----
 app/test-pmd/txonly.c                       | 35 +++++++++++++++++++--
 doc/guides/testpmd_app_ug/testpmd_funcs.rst | 13 ++++++++
 3 files changed, 51 insertions(+), 9 deletions(-)

diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 9d457ca88e..46ff678b9f 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -6327,9 +6327,6 @@ set_tx_pkt_segments(unsigned int *seg_lengths, unsigned int nb_segs)
 	/*
 	 * Check that each segment length is greater or equal than
 	 * the mbuf data size.
-	 * Check also that the total packet length is greater or equal than the
-	 * size of an empty UDP/IP packet (sizeof(struct rte_ether_hdr) +
-	 * 20 + 8).
 	 */
 	tx_pkt_len = 0;
 	for (i = 0; i < nb_segs; i++) {
@@ -6341,10 +6338,11 @@ set_tx_pkt_segments(unsigned int *seg_lengths, unsigned int nb_segs)
 		}
 		tx_pkt_len = (uint16_t)(tx_pkt_len + seg_lengths[i]);
 	}
-	if (tx_pkt_len < (sizeof(struct rte_ether_hdr) + 20 + 8)) {
-		fprintf(stderr, "total packet length=%u < %d - give up\n",
-				(unsigned) tx_pkt_len,
-				(int)(sizeof(struct rte_ether_hdr) + 20 + 8));
+
+	/* Allow runt packets this is a test tool. */
+	if (tx_pkt_len < (sizeof(struct rte_ether_hdr))) {
+		fprintf(stderr, "total packet length=%u < %zu - give up\n",
+			tx_pkt_len, sizeof(struct rte_ether_hdr));
 		return;
 	}
 
diff --git a/app/test-pmd/txonly.c b/app/test-pmd/txonly.c
index 64893fa205..2e0abe361e 100644
--- a/app/test-pmd/txonly.c
+++ b/app/test-pmd/txonly.c
@@ -76,6 +76,12 @@ copy_buf_to_pkt_segs(void* buf, unsigned len, struct rte_mbuf *pkt,
 	while (offset >= seg->data_len) {
 		offset -= seg->data_len;
 		seg = seg->next;
+		/*
+		 * The packet may be shorter than the header stack when
+		 * generating runt frames; stop once it runs out of segments.
+		 */
+		if (seg == NULL)
+			return;
 	}
 	copy_len = seg->data_len - offset;
 	seg_buf = rte_pktmbuf_mtod_offset(seg, char *, offset);
@@ -84,6 +90,8 @@ copy_buf_to_pkt_segs(void* buf, unsigned len, struct rte_mbuf *pkt,
 		len -= copy_len;
 		buf = ((char*) buf + copy_len);
 		seg = seg->next;
+		if (seg == NULL)
+			return;
 		seg_buf = rte_pktmbuf_mtod(seg, char *);
 		copy_len = seg->data_len;
 	}
@@ -193,7 +201,6 @@ pkt_burst_prepare(struct rte_mbuf *pkt, struct rte_mempool *mbp,
 	pkt->vlan_tci = vlan_tci;
 	pkt->vlan_tci_outer = vlan_tci_outer;
 	pkt->l2_len = sizeof(struct rte_ether_hdr);
-	pkt->l3_len = sizeof(struct rte_ipv4_hdr);
 
 	pkt_len = pkt->data_len;
 	pkt_seg = pkt;
@@ -204,6 +211,24 @@ pkt_burst_prepare(struct rte_mbuf *pkt, struct rte_mempool *mbp,
 		pkt_len += pkt_seg->data_len;
 	}
 	pkt_seg->next = NULL; /* Last segment of packet. */
+
+	/*
+	 * A runt frame may be too short to carry a full IPv4/UDP header.
+	 * Clamp l3_len and drop any checksum offload whose header is not
+	 * fully present, so the PMD is never asked to checksum bytes that
+	 * are not in the frame. pkt_len is at least sizeof(rte_ether_hdr),
+	 * so the subtraction below cannot underflow.
+	 */
+	pkt->l3_len = RTE_MIN(sizeof(struct rte_ipv4_hdr),
+			      pkt_len - sizeof(struct rte_ether_hdr));
+	if (pkt_len < sizeof(struct rte_ether_hdr) +
+			sizeof(struct rte_ipv4_hdr))
+		pkt->ol_flags &= ~(RTE_MBUF_F_TX_IP_CKSUM |
+				   RTE_MBUF_F_TX_L4_MASK);
+	else if (pkt_len < sizeof(struct rte_ether_hdr) +
+			sizeof(struct rte_ipv4_hdr) +
+			sizeof(struct rte_udp_hdr))
+		pkt->ol_flags &= ~RTE_MBUF_F_TX_L4_MASK;
 	/*
 	 * Copy headers in first packet segment(s).
 	 */
@@ -405,7 +430,13 @@ tx_only_begin(portid_t pi)
 	pkt_hdr_len = (uint16_t)(sizeof(struct rte_ether_hdr) +
 				 sizeof(struct rte_ipv4_hdr) +
 				 sizeof(struct rte_udp_hdr));
-	pkt_data_len = tx_pkt_length - pkt_hdr_len;
+	/*
+	 * tx_pkt_length may be smaller than the full header stack when
+	 * generating runt frames; clamp the payload length to zero in that
+	 * case so the IP/UDP length fields stay sane.
+	 */
+	pkt_data_len = tx_pkt_length > pkt_hdr_len ?
+			tx_pkt_length - pkt_hdr_len : 0;
 
 	if ((tx_pkt_split == TX_PKT_SPLIT_RND || txonly_multi_flow) &&
 	    tx_pkt_seg_lengths[0] < pkt_hdr_len) {
diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index f0f2b0758b..d2e5b63586 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -876,6 +876,19 @@ Set the length of each segment of the TX-ONLY packets or length of packet for FL
 
 Where x[,y]* represents a CSV list of values, without white space.
 
+The total packet length may be set as small as the Ethernet header
+(``sizeof(struct rte_ether_hdr)``), which is below the size of an empty
+UDP/IPv4 packet. This generates runt frames with a truncated (or absent)
+IPv4/UDP header, which is useful for testing how a driver handles
+undersized frames. Any header bytes that do not fit in the requested
+length are simply not emitted. Checksum offloads are automatically
+dropped for a frame too short to contain the corresponding header.
+
+Note that random split (``set txsplit rand``) and multi-flow
+(``set txonly-flows``) still require the first segment to hold the full
+Ethernet/IPv4/UDP header stack, so they cannot be combined with runt
+lengths.
+
 set txtimes
 ~~~~~~~~~~~
 
-- 
2.53.0


^ permalink raw reply related

* [PATCH v1] tools: AI review handle empty Error sections
From: Matthew Gee @ 2026-06-12 19:02 UTC (permalink / raw)
  To: dev; +Cc: stephen, aconole, lylavoie, Matthew Gee

Previous review-patch.py would detect and report and error or warning
based off of the occurrence of the headers of the error and warning
sections. This led to consistent false positives as often AI reviewers
will create the header but put "none" or similar filler text within
the following body.

This patch updates the code in order to check if the AI review has a
body with error or warnings to fix and not just filler text. This is
done by querying multiple lines at once and adjusting the regex to
filter out headers followed only by filler text or formatting in the
review.

These changes were tested against 10+ AI review outputs with several
variations in formatting and filler text in order to catch a good
variety of cases to make sure code reviews with actual errors or
warnings are caught and not missed.

Signed-off-by: Matthew Gee <mgee@iol.unh.edu>
---
 devtools/ai/review-patch.py | 35 +++++++++++++++++++++++++++--------
 1 file changed, 27 insertions(+), 8 deletions(-)

diff --git a/devtools/ai/review-patch.py b/devtools/ai/review-patch.py
index 52601ac156..b009368638 100755
--- a/devtools/ai/review-patch.py
+++ b/devtools/ai/review-patch.py
@@ -19,6 +19,7 @@
 from email.message import EmailMessage
 from pathlib import Path
 from typing import Any, Iterator
+from itertools import tee, islice, chain
 
 from _common import (
     PROVIDERS,
@@ -147,19 +148,37 @@ def classify_review(review_text: str, output_format: str) -> int:
             pass  # Fall through to text scanning
 
     if not has_errors and not has_warnings:
-        # Scan review text for severity indicators.
-        # Match section headers and inline markers across text/markdown/html.
-        for line in review_text.splitlines():
-            stripped = line.strip().lower()
+        # Matches against error or warning section headers
+        rgx_should_match: str = r"#+\s(\*+)?{err_or_warn}"
+        # Matches against headers followed only by filler text, formatting, or no additional text
+        rgx_should_not_match: str = r"#+\s(\*+)?{err_or_warn}(s?)(\*+)?(none(.)?$| \(must fix\)$|$)"
+        
+        curr_lines: iter[str]
+        next_lines: iter[str | None]
+        next_next_lines: iter[str | None]
+        curr_lines, next_lines, next_next_lines = tee(review_text.splitlines(), 3)
+        next_lines = chain(islice(next_lines, 1, None), [None])
+        next_next_lines = chain(islice(next_next_lines, 2, None), [None, None])
+        
+        curr_lines: str
+        next_lines: str | None
+        next_next_lines: str | None
+        for curr_line, next_line, next_next_line in zip(curr_lines, next_lines, next_next_lines):
+            stripped: str = curr_line.strip().lower() + str(
+                next_line or '').strip().lower() + str(next_next_line or '').strip().lower()
             # Skip quoted patch context lines
             if stripped.startswith(">") or stripped.startswith("diff --git"):
                 continue
-            if re.match(r"^(#{1,3}\s+)?(\*{0,2})error", stripped) or re.match(
-                r"^<h[1-3]>\s*error", stripped
+
+            elif re.match(rgx_should_match.format(err_or_warn='error'),
+                          stripped) and not re.match(
+                rgx_should_not_match.format(err_or_warn='error'), stripped
             ):
                 has_errors = True
-            elif re.match(r"^(#{1,3}\s+)?(\*{0,2})warning", stripped) or re.match(
-                r"^<h[1-3]>\s*warning", stripped
+
+            elif re.match(rgx_should_match.format(err_or_warn='warning'),
+                          stripped) and not re.match(
+                rgx_should_not_match.format(err_or_warn='warning'), stripped
             ):
                 has_warnings = True
 
-- 
2.54.0


^ permalink raw reply related

* [PATCH v2] tools: AI review handle empty Error sections
From: Matthew Gee @ 2026-06-12 19:08 UTC (permalink / raw)
  To: dev; +Cc: stephen, aconole, lylavoie, Matthew Gee
In-Reply-To: <20260612190225.1016275-1-mgee@iol.unh.edu>

Previous review-patch.py would detect and report and error or warning
based off of the occurrence of the headers of the error and warning
sections. This led to consistent false positives as often AI reviewers
will create the header but put "none" or similar filler text within
the following body.

This patch updates the code in order to check if the AI review has a
body with error or warnings to fix and not just filler text. This is
done by querying multiple lines at once and adjusting the regex to
filter out headers followed only by filler text or formatting in the
review.

These changes were tested against 10+ AI review outputs with several
variations in formatting and filler text in order to catch a good
variety of cases to make sure code reviews with actual errors or
warnings are caught and not missed.

Signed-off-by: Matthew Gee <mgee@iol.unh.edu>
---
 devtools/ai/review-patch.py | 35 +++++++++++++++++++++++++++--------
 1 file changed, 27 insertions(+), 8 deletions(-)

diff --git a/devtools/ai/review-patch.py b/devtools/ai/review-patch.py
index 52601ac156..eb608566d2 100755
--- a/devtools/ai/review-patch.py
+++ b/devtools/ai/review-patch.py
@@ -19,6 +19,7 @@
 from email.message import EmailMessage
 from pathlib import Path
 from typing import Any, Iterator
+from itertools import tee, islice, chain
 
 from _common import (
     PROVIDERS,
@@ -147,19 +148,37 @@ def classify_review(review_text: str, output_format: str) -> int:
             pass  # Fall through to text scanning
 
     if not has_errors and not has_warnings:
-        # Scan review text for severity indicators.
-        # Match section headers and inline markers across text/markdown/html.
-        for line in review_text.splitlines():
-            stripped = line.strip().lower()
+        # Matches against error or warning section headers
+        rgx_should_match: str = r"#+\s(\*+)?{err_or_warn}"
+        # Matches against headers followed only by filler text, formatting, or no additional text
+        rgx_should_not_match: str = r"#+\s(\*+)?{err_or_warn}(s?)(\*+)?(none(.)?$| \(must fix\)$|$)"
+
+        curr_lines: iter[str]
+        next_lines: iter[str | None]
+        next_next_lines: iter[str | None]
+        curr_lines, next_lines, next_next_lines = tee(review_text.splitlines(), 3)
+        next_lines = chain(islice(next_lines, 1, None), [None])
+        next_next_lines = chain(islice(next_next_lines, 2, None), [None, None])
+
+        curr_lines: str
+        next_lines: str | None
+        next_next_lines: str | None
+        for curr_line, next_line, next_next_line in zip(curr_lines, next_lines, next_next_lines):
+            stripped: str = curr_line.strip().lower() + str(
+                next_line or '').strip().lower() + str(next_next_line or '').strip().lower()
             # Skip quoted patch context lines
             if stripped.startswith(">") or stripped.startswith("diff --git"):
                 continue
-            if re.match(r"^(#{1,3}\s+)?(\*{0,2})error", stripped) or re.match(
-                r"^<h[1-3]>\s*error", stripped
+
+            elif re.match(rgx_should_match.format(err_or_warn='error'),
+                          stripped) and not re.match(
+                rgx_should_not_match.format(err_or_warn='error'), stripped
             ):
                 has_errors = True
-            elif re.match(r"^(#{1,3}\s+)?(\*{0,2})warning", stripped) or re.match(
-                r"^<h[1-3]>\s*warning", stripped
+
+            elif re.match(rgx_should_match.format(err_or_warn='warning'),
+                          stripped) and not re.match(
+                rgx_should_not_match.format(err_or_warn='warning'), stripped
             ):
                 has_warnings = True
 
-- 
2.54.0


^ permalink raw reply related


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