[PATCH 0/3] bpf: simple tests and fixes

[PATCH 0/3] bpf: simple tests and fixes

[Marat Khalili]

Add simple unit-style tests for rte_bpf_load, and fix some minor
discovered bugs.

Create new source file for new tests since app/test/test_bpf.c was
growing rather large, and tests there look more like functional rather
than unit ones.

Marat Khalili (3):
  bpf: fix signed shift overflows in ARM JIT
  bpf: disallow empty program
  bpf: make add/subtract one program validate

 app/test/meson.build       |   1 +
 app/test/test_bpf_simple.c | 189 +++++++++++++++++++++++++++++++++++++
 lib/bpf/bpf_jit_arm64.c    | 188 ++++++++++++++++++++----------------
 lib/bpf/bpf_load.c         |   2 +-
 lib/bpf/bpf_validate.c     |  28 ++++--
 5 files changed, 318 insertions(+), 90 deletions(-)
 create mode 100644 app/test/test_bpf_simple.c

-- 
2.43.0

[PATCH 1/3] bpf: fix signed shift overflows in ARM JIT

[Marat Khalili]

Left shifts of integer literals and bool values overwriting the sign bit
were used multiple times in bpf_jit_arm64.c. E.g.:

    insn = (!!is64) << 31;

where is64 has type bool (double bang is a no-op here). The operand of
left shift was promoted to type int, which when 32-bit wide cannot
represent the result. Similarly literal integers have int type by
default.  Sanitizer produced the following diagnostic during runtime
(for various lines):

    lib/bpf/bpf_jit_arm64.c:241:18: runtime error: left shift of 1 by 31
    places cannot be represented in type 'int'

To fix the issue use RTE_BIT32 and similar macros instead.

Signed-off-by: Marat Khalili <marat.khalili@huawei.com>
---
 lib/bpf/bpf_jit_arm64.c | 188 +++++++++++++++++++++++-----------------
 1 file changed, 107 insertions(+), 81 deletions(-)

diff --git a/lib/bpf/bpf_jit_arm64.c b/lib/bpf/bpf_jit_arm64.c
index 96b8cd2e03..5f43db0170 100644
--- a/lib/bpf/bpf_jit_arm64.c
+++ b/lib/bpf/bpf_jit_arm64.c
@@ -28,7 +28,33 @@
 #define A64_ZR			31
 
 #define check_imm(n, val) (((val) >= 0) ? !!((val) >> (n)) : !!((~val) >> (n)))
-#define mask_imm(n, val) ((val) & ((1 << (n)) - 1))
+#define mask_imm(n, val) ((val) & (RTE_BIT32(n) - 1))
+
+/**
+ * Get the uint32_t shifted value.
+ *
+ * Works similarly to RTE_SHIFT_VAL32 but accepts non-literal arguments.
+ * Performs identically to RTE_SHIFT_VAL32 with literal arguments.
+ *
+ * @param val
+ *   The value to be shifted, can be non-literal.
+ * @param nr
+ *   The shift number in range of 0 to (32 - width of val).
+ */
+#define SHIFT_VAR32(val, nr) ((uint32_t)(val) << (nr))
+
+/**
+ * Get the uint64_t shifted value.
+ *
+ * Works similarly to RTE_SHIFT_VAL64 but accepts non-literal arguments.
+ * Performs identically to RTE_SHIFT_VAL64 with literal arguments.
+ *
+ * @param val
+ *   The value to be shifted, can be non-literal.
+ * @param nr
+ *   The shift number in range of 0 to (64 - width of val).
+ */
+#define SHIFT_VAR64(val, nr) ((uint64_t)(val) << (nr))
 
 struct ebpf_a64_map {
 	uint32_t off; /* eBPF to arm64 insn offset mapping for jump */
@@ -238,12 +264,12 @@ emit_add_sub_imm(struct a64_jit_ctx *ctx, bool is64, bool sub, uint8_t rd,
 	uint32_t insn, imm;
 
 	imm = mask_imm(12, imm12);
-	insn = (!!is64) << 31;
-	insn |= (!!sub) << 30;
+	insn = SHIFT_VAR32(is64, 31);
+	insn |= SHIFT_VAR32(sub, 30);
 	insn |= 0x11000000;
 	insn |= rd;
-	insn |= rn << 5;
-	insn |= imm << 10;
+	insn |= SHIFT_VAR32(rn, 5);
+	insn |= SHIFT_VAR32(imm, 10);
 
 	emit_insn(ctx, insn,
 		  check_reg(rd) || check_reg(rn) || check_imm(12, imm12));
@@ -279,16 +305,16 @@ emit_ls_pair_64(struct a64_jit_ctx *ctx, uint8_t rt, uint8_t rt2, uint8_t rn,
 {
 	uint32_t insn;
 
-	insn = (!!load) << 22;
-	insn |= (!!pre_index) << 24;
+	insn = SHIFT_VAR32(load, 22);
+	insn |= SHIFT_VAR32(pre_index, 24);
 	insn |= 0xa8800000;
 	insn |= rt;
-	insn |= rn << 5;
-	insn |= rt2 << 10;
+	insn |= SHIFT_VAR32(rn, 5);
+	insn |= SHIFT_VAR32(rt2, 10);
 	if (push)
-		insn |= 0x7e << 15; /* 0x7e means -2 with imm7 */
+		insn |= SHIFT_VAR32(0x7e, 15); /* 0x7e means -2 with imm7 */
 	else
-		insn |= 0x2 << 15;
+		insn |= SHIFT_VAR32(0x2, 15);
 
 	emit_insn(ctx, insn, check_reg(rn) || check_reg(rt) || check_reg(rt2));
 
@@ -317,11 +343,11 @@ mov_imm(struct a64_jit_ctx *ctx, bool is64, uint8_t rd, uint8_t type,
 {
 	uint32_t insn;
 
-	insn = (!!is64) << 31;
-	insn |= type << 29;
-	insn |= 0x25 << 23;
-	insn |= (shift/16) << 21;
-	insn |= imm16 << 5;
+	insn = SHIFT_VAR32(is64, 31);
+	insn |= SHIFT_VAR32(type, 29);
+	insn |= SHIFT_VAR32(0x25, 23);
+	insn |= SHIFT_VAR32(shift/16, 21);
+	insn |= SHIFT_VAR32(imm16, 5);
 	insn |= rd;
 
 	emit_insn(ctx, insn, check_reg(rd) || check_mov_hw(is64, shift));
@@ -334,7 +360,7 @@ emit_mov_imm32(struct a64_jit_ctx *ctx, bool is64, uint8_t rd, uint32_t val)
 	uint16_t lower = val & 0xffff;
 
 	/* Positive number */
-	if ((val & 1UL << 31) == 0) {
+	if ((val & RTE_BIT32(31)) == 0) {
 		mov_imm(ctx, is64, rd, A64_MOVZ, lower, 0);
 		if (upper)
 			mov_imm(ctx, is64, rd, A64_MOVK, upper, 16);
@@ -393,21 +419,21 @@ emit_ls(struct a64_jit_ctx *ctx, uint8_t sz, uint8_t rt, uint8_t rn, uint8_t rm,
 {
 	uint32_t insn;
 
-	insn = 0x1c1 << 21;
+	insn = SHIFT_VAR32(0x1c1, 21);
 	if (load)
-		insn |= 1 << 22;
+		insn |= RTE_BIT32(22);
 	if (sz == BPF_B)
-		insn |= 0 << 30;
+		insn |= SHIFT_VAR32(0, 30);
 	else if (sz == BPF_H)
-		insn |= 1 << 30;
+		insn |= SHIFT_VAR32(1, 30);
 	else if (sz == BPF_W)
-		insn |= 2 << 30;
+		insn |= SHIFT_VAR32(2, 30);
 	else if (sz == EBPF_DW)
-		insn |= 3 << 30;
+		insn |= SHIFT_VAR32(3, 30);
 
-	insn |= rm << 16;
-	insn |= 0x1a << 10; /* LSL and S = 0 */
-	insn |= rn << 5;
+	insn |= SHIFT_VAR32(rm, 16);
+	insn |= SHIFT_VAR32(0x1a, 10); /* LSL and S = 0 */
+	insn |= SHIFT_VAR32(rn, 5);
 	insn |= rt;
 
 	emit_insn(ctx, insn, check_reg(rt) || check_reg(rn) || check_reg(rm) ||
@@ -436,10 +462,10 @@ emit_add_sub(struct a64_jit_ctx *ctx, bool is64, uint8_t rd, uint8_t rn,
 {
 	uint32_t insn;
 
-	insn = (!!is64) << 31;
-	insn |= op << 21; /* shift == 0 */
-	insn |= rm << 16;
-	insn |= rn << 5;
+	insn = SHIFT_VAR32(is64, 31);
+	insn |= SHIFT_VAR32(op, 21); /* shift == 0 */
+	insn |= SHIFT_VAR32(rm, 16);
+	insn |= SHIFT_VAR32(rn, 5);
 	insn |= rd;
 
 	emit_insn(ctx, insn, check_reg(rd) || check_reg(rm));
@@ -468,11 +494,11 @@ emit_mul(struct a64_jit_ctx *ctx, bool is64, uint8_t rd, uint8_t rm)
 {
 	uint32_t insn;
 
-	insn = (!!is64) << 31;
-	insn |= 0xd8 << 21;
-	insn |= rm << 16;
-	insn |= A64_ZR << 10;
-	insn |= rd << 5;
+	insn = SHIFT_VAR32(is64, 31);
+	insn |= SHIFT_VAR32(0xd8, 21);
+	insn |= SHIFT_VAR32(rm, 16);
+	insn |= SHIFT_VAR32(A64_ZR, 10);
+	insn |= SHIFT_VAR32(rd, 5);
 	insn |= rd;
 
 	emit_insn(ctx, insn, check_reg(rd) || check_reg(rm));
@@ -489,11 +515,11 @@ emit_data_process_two_src(struct a64_jit_ctx *ctx, bool is64, uint8_t rd,
 {
 	uint32_t insn;
 
-	insn = (!!is64) << 31;
-	insn |= 0xd6 << 21;
-	insn |= rm << 16;
-	insn |= op << 10;
-	insn |= rn << 5;
+	insn = SHIFT_VAR32(is64, 31);
+	insn |= SHIFT_VAR32(0xd6, 21);
+	insn |= SHIFT_VAR32(rm, 16);
+	insn |= SHIFT_VAR32(op, 10);
+	insn |= SHIFT_VAR32(rn, 5);
 	insn |= rd;
 
 	emit_insn(ctx, insn, check_reg(rd) || check_reg(rm));
@@ -532,14 +558,14 @@ emit_bitfield(struct a64_jit_ctx *ctx, bool is64, uint8_t rd, uint8_t rn,
 {
 	uint32_t insn;
 
-	insn = (!!is64) << 31;
+	insn = SHIFT_VAR32(is64, 31);
 	if (insn)
-		insn |= 1 << 22; /* Set N bit when is64 is set */
-	insn |= op << 29;
-	insn |= 0x26 << 23;
-	insn |= immr << 16;
-	insn |= imms << 10;
-	insn |= rn << 5;
+		insn |= RTE_BIT32(22); /* Set N bit when is64 is set */
+	insn |= SHIFT_VAR32(op, 29);
+	insn |= SHIFT_VAR32(0x26, 23);
+	insn |= SHIFT_VAR32(immr, 16);
+	insn |= SHIFT_VAR32(imms, 10);
+	insn |= SHIFT_VAR32(rn, 5);
 	insn |= rd;
 
 	emit_insn(ctx, insn, check_reg(rd) || check_reg(rn) ||
@@ -578,11 +604,11 @@ emit_logical(struct a64_jit_ctx *ctx, bool is64, uint8_t rd,
 {
 	uint32_t insn;
 
-	insn = (!!is64) << 31;
-	insn |= op << 29;
-	insn |= 0x50 << 21;
-	insn |= rm << 16;
-	insn |= rd << 5;
+	insn = SHIFT_VAR32(is64, 31);
+	insn |= SHIFT_VAR32(op, 29);
+	insn |= SHIFT_VAR32(0x50, 21);
+	insn |= SHIFT_VAR32(rm, 16);
+	insn |= SHIFT_VAR32(rd, 5);
 	insn |= rd;
 
 	emit_insn(ctx, insn, check_reg(rd) || check_reg(rm));
@@ -612,12 +638,12 @@ emit_msub(struct a64_jit_ctx *ctx, bool is64, uint8_t rd, uint8_t rn,
 {
 	uint32_t insn;
 
-	insn = (!!is64) << 31;
-	insn |= 0xd8 << 21;
-	insn |= rm << 16;
-	insn |= 0x1 << 15;
-	insn |= ra << 10;
-	insn |= rn << 5;
+	insn = SHIFT_VAR32(is64, 31);
+	insn |= SHIFT_VAR32(0xd8, 21);
+	insn |= SHIFT_VAR32(rm, 16);
+	insn |= SHIFT_VAR32(0x1, 15);
+	insn |= SHIFT_VAR32(ra, 10);
+	insn |= SHIFT_VAR32(rn, 5);
 	insn |= rd;
 
 	emit_insn(ctx, insn, check_reg(rd) || check_reg(rn) || check_reg(rm) ||
@@ -638,7 +664,7 @@ emit_blr(struct a64_jit_ctx *ctx, uint8_t rn)
 	uint32_t insn;
 
 	insn = 0xd63f0000;
-	insn |= rn << 5;
+	insn |= SHIFT_VAR32(rn, 5);
 
 	emit_insn(ctx, insn, check_reg(rn));
 }
@@ -669,22 +695,22 @@ emit_rev(struct a64_jit_ctx *ctx, uint8_t rd, int32_t imm)
 	uint32_t insn;
 
 	insn = 0xdac00000;
-	insn |= rd << 5;
+	insn |= SHIFT_VAR32(rd, 5);
 	insn |= rd;
 
 	switch (imm) {
 	case 16:
-		insn |= 1 << 10;
+		insn |= SHIFT_VAR32(1, 10);
 		emit_insn(ctx, insn, check_reg(rd));
 		emit_zero_extend(ctx, rd, 16);
 		break;
 	case 32:
-		insn |= 2 << 10;
+		insn |= SHIFT_VAR32(2, 10);
 		emit_insn(ctx, insn, check_reg(rd));
 		/* Upper 32 bits already cleared */
 		break;
 	case 64:
-		insn |= 3 << 10;
+		insn |= SHIFT_VAR32(3, 10);
 		emit_insn(ctx, insn, check_reg(rd));
 		break;
 	default:
@@ -933,9 +959,9 @@ emit_cbnz(struct a64_jit_ctx *ctx, bool is64, uint8_t rt, int32_t imm19)
 	uint32_t insn, imm;
 
 	imm = mask_imm(19, imm19);
-	insn = (!!is64) << 31;
-	insn |= 0x35 << 24;
-	insn |= imm << 5;
+	insn = SHIFT_VAR32(is64, 31);
+	insn |= SHIFT_VAR32(0x35, 24);
+	insn |= SHIFT_VAR32(imm, 5);
 	insn |= rt;
 
 	emit_insn(ctx, insn, check_reg(rt) || check_imm(19, imm19));
@@ -947,7 +973,7 @@ emit_b(struct a64_jit_ctx *ctx, int32_t imm26)
 	uint32_t insn, imm;
 
 	imm = mask_imm(26, imm26);
-	insn = 0x5 << 26;
+	insn = SHIFT_VAR32(0x5, 26);
 	insn |= imm;
 
 	emit_insn(ctx, insn, check_imm(26, imm26));
@@ -971,9 +997,9 @@ emit_stadd(struct a64_jit_ctx *ctx, bool is64, uint8_t rs, uint8_t rn)
 	uint32_t insn;
 
 	insn = 0xb820001f;
-	insn |= (!!is64) << 30;
-	insn |= rs << 16;
-	insn |= rn << 5;
+	insn |= SHIFT_VAR32(is64, 30);
+	insn |= SHIFT_VAR32(rs, 16);
+	insn |= SHIFT_VAR32(rn, 5);
 
 	emit_insn(ctx, insn, check_reg(rs) || check_reg(rn));
 }
@@ -984,8 +1010,8 @@ emit_ldxr(struct a64_jit_ctx *ctx, bool is64, uint8_t rt, uint8_t rn)
 	uint32_t insn;
 
 	insn = 0x885f7c00;
-	insn |= (!!is64) << 30;
-	insn |= rn << 5;
+	insn |= SHIFT_VAR32(is64, 30);
+	insn |= SHIFT_VAR32(rn, 5);
 	insn |= rt;
 
 	emit_insn(ctx, insn, check_reg(rt) || check_reg(rn));
@@ -998,9 +1024,9 @@ emit_stxr(struct a64_jit_ctx *ctx, bool is64, uint8_t rs, uint8_t rt,
 	uint32_t insn;
 
 	insn = 0x88007c00;
-	insn |= (!!is64) << 30;
-	insn |= rs << 16;
-	insn |= rn << 5;
+	insn |= SHIFT_VAR32(is64, 30);
+	insn |= SHIFT_VAR32(rs, 16);
+	insn |= SHIFT_VAR32(rn, 5);
 	insn |= rt;
 
 	emit_insn(ctx, insn, check_reg(rs) || check_reg(rt) || check_reg(rn));
@@ -1051,9 +1077,9 @@ emit_cmp_tst(struct a64_jit_ctx *ctx, bool is64, uint8_t rn, uint8_t rm,
 	uint32_t insn;
 
 	insn = opc;
-	insn |= (!!is64) << 31;
-	insn |= rm << 16;
-	insn |= rn << 5;
+	insn |= SHIFT_VAR32(is64, 31);
+	insn |= SHIFT_VAR32(rm, 16);
+	insn |= SHIFT_VAR32(rn, 5);
 
 	emit_insn(ctx, insn, check_reg(rn) || check_reg(rm));
 }
@@ -1076,8 +1102,8 @@ emit_b_cond(struct a64_jit_ctx *ctx, uint8_t cond, int32_t imm19)
 	uint32_t insn, imm;
 
 	imm = mask_imm(19, imm19);
-	insn = 0x15 << 26;
-	insn |= imm << 5;
+	insn = SHIFT_VAR32(0x15, 26);
+	insn |= SHIFT_VAR32(imm, 5);
 	insn |= cond;
 
 	emit_insn(ctx, insn, check_cond(cond) || check_imm(19, imm19));
@@ -1301,7 +1327,7 @@ emit(struct a64_jit_ctx *ctx, struct rte_bpf *bpf)
 			break;
 		/* dst = imm64 */
 		case (BPF_LD | BPF_IMM | EBPF_DW):
-			u64 = ((uint64_t)ins[1].imm << 32) | (uint32_t)imm;
+			u64 = SHIFT_VAR64(ins[1].imm, 32) | (uint32_t)imm;
 			emit_mov_imm(ctx, 1, dst, u64);
 			i++;
 			break;
-- 
2.43.0

[PATCH 2/3] bpf: disallow empty program

[Marat Khalili]

Add tests for some simple cases:
* Program with no instructions;
* Program with only EXIT instruction but no return value set;
* Program with return value set but no EXIT instruction;
* Minimal valid program with return value set and an EXIT instruction.

Fix found bugs:
* a program with no instructions was accepted;
* a program with no EXIT instruction read outside the buffer.

Signed-off-by: Marat Khalili <marat.khalili@huawei.com>
---
 app/test/meson.build       |   1 +
 app/test/test_bpf_simple.c | 131 +++++++++++++++++++++++++++++++++++++
 lib/bpf/bpf_load.c         |   2 +-
 lib/bpf/bpf_validate.c     |  20 ++++--
 4 files changed, 149 insertions(+), 5 deletions(-)
 create mode 100644 app/test/test_bpf_simple.c

diff --git a/app/test/meson.build b/app/test/meson.build
index 8df8d3edd1..9d48431ba0 100644
--- a/app/test/meson.build
+++ b/app/test/meson.build
@@ -35,6 +35,7 @@ source_file_deps = {
     'test_bitset.c': [],
     'test_bitratestats.c': ['metrics', 'bitratestats', 'ethdev'] + sample_packet_forward_deps,
     'test_bpf.c': ['bpf', 'net'],
+    'test_bpf_simple.c': ['bpf'],
     'test_byteorder.c': [],
     'test_cfgfile.c': ['cfgfile'],
     'test_cksum.c': ['net'],
diff --git a/app/test/test_bpf_simple.c b/app/test/test_bpf_simple.c
new file mode 100644
index 0000000000..576a6ed029
--- /dev/null
+++ b/app/test/test_bpf_simple.c
@@ -0,0 +1,131 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2025 Huawei Technologies Co., Ltd
+ */
+
+#include "test.h"
+
+#include <rte_bpf.h>
+#include <rte_errno.h>
+
+/* Tests of most simple BPF programs (no instructions, one instruction etc.) */
+
+/*
+ * Try to load a simple bpf program from the instructions array.
+ *
+ * When `expected_errno` is zero, expect it to load successfully.
+ * When `expected_errno` is non-zero, expect it to fail with this `rte_errno`.
+ *
+ * @param nb_ins
+ *   Number of instructions in the `ins` array.
+ * @param ins
+ *   BPF instructions array.
+ * @param expected_errno
+ *   Expected result.
+ * @return
+ *   TEST_SUCCESS on success, error code on failure.
+ */
+static int
+simple_bpf_load_test(uint32_t nb_ins, const struct ebpf_insn *ins,
+	int expected_errno)
+{
+	const struct rte_bpf_prm prm = {
+		.ins = ins,
+		.nb_ins = nb_ins,
+		.prog_arg = {
+			.type = RTE_BPF_ARG_RAW,
+			.size = sizeof(uint64_t),
+		},
+	};
+
+	struct rte_bpf *const bpf = rte_bpf_load(&prm);
+	const int actual_errno = rte_errno;
+	rte_bpf_destroy(bpf);
+
+	if (expected_errno != 0) {
+		RTE_TEST_ASSERT_EQUAL(bpf, NULL,
+			"expect rte_bpf_load() == NULL");
+		RTE_TEST_ASSERT_EQUAL(actual_errno, expected_errno,
+			"expect rte_errno == %d, found %d",
+			expected_errno, actual_errno);
+	} else
+		RTE_TEST_ASSERT_NOT_EQUAL(bpf, NULL,
+			"expect rte_bpf_load() != NULL");
+
+	return TEST_SUCCESS;
+}
+
+/*
+ * Try and load completely empty BPF program.
+ * Should fail because there is no EXIT (and also return value is undefined).
+ */
+static int
+test_simple_no_instructions(void)
+{
+	static const struct ebpf_insn ins[] = {};
+	return simple_bpf_load_test(RTE_DIM(ins), ins, EINVAL);
+}
+
+REGISTER_FAST_TEST(bpf_simple_no_instructions_autotest, true, true,
+	test_simple_no_instructions);
+
+/*
+ * Try and load a BPF program comprising single EXIT instruction.
+ * Should fail because the return value is undefined.
+ */
+static int
+test_simple_exit_only(void)
+{
+	static const struct ebpf_insn ins[] = {
+		{
+			.code = (BPF_JMP | EBPF_EXIT),
+		},
+	};
+	return simple_bpf_load_test(RTE_DIM(ins), ins, EINVAL);
+}
+
+REGISTER_FAST_TEST(bpf_simple_exit_only_autotest, true, true,
+	test_simple_exit_only);
+
+/*
+ * Try and load a BPF program with no EXIT instruction.
+ * Should fail because of this.
+ */
+static int
+test_simple_no_exit(void)
+{
+	static const struct ebpf_insn ins[] = {
+		{
+			/* Set return value to the program argument. */
+			.code = (EBPF_ALU64 | EBPF_MOV | BPF_X),
+			.src_reg = EBPF_REG_1,
+			.dst_reg = EBPF_REG_0,
+		},
+	};
+	return simple_bpf_load_test(RTE_DIM(ins), ins, EINVAL);
+}
+
+REGISTER_FAST_TEST(bpf_simple_no_exit_autotest, true, true,
+	test_simple_no_exit);
+
+/*
+ * Try and load smallest possible valid BPF program.
+ */
+static int
+test_simple_minimal_working(void)
+{
+	static const struct ebpf_insn ins[] = {
+		{
+			/* Set return value to the program argument. */
+			.code = (EBPF_ALU64 | EBPF_MOV | BPF_X),
+			.src_reg = EBPF_REG_1,
+			.dst_reg = EBPF_REG_0,
+		},
+		{
+			.code = (BPF_JMP | EBPF_EXIT),
+		},
+	};
+	return simple_bpf_load_test(RTE_DIM(ins), ins, 0);
+}
+
+REGISTER_FAST_TEST(bpf_simple_minimal_working_autotest, true, true,
+	test_simple_minimal_working);
diff --git a/lib/bpf/bpf_load.c b/lib/bpf/bpf_load.c
index 556e613762..6983c026af 100644
--- a/lib/bpf/bpf_load.c
+++ b/lib/bpf/bpf_load.c
@@ -88,7 +88,7 @@ rte_bpf_load(const struct rte_bpf_prm *prm)
 	int32_t rc;
 	uint32_t i;
 
-	if (prm == NULL || prm->ins == NULL ||
+	if (prm == NULL || prm->ins == NULL || prm->nb_ins == 0 ||
 			(prm->nb_xsym != 0 && prm->xsym == NULL)) {
 		rte_errno = EINVAL;
 		return NULL;
diff --git a/lib/bpf/bpf_validate.c b/lib/bpf/bpf_validate.c
index 4f47d6dc7b..23444b3eaa 100644
--- a/lib/bpf/bpf_validate.c
+++ b/lib/bpf/bpf_validate.c
@@ -1827,7 +1827,7 @@ add_edge(struct bpf_verifier *bvf, struct inst_node *node, uint32_t nidx)
 {
 	uint32_t ne;
 
-	if (nidx > bvf->prm->nb_ins) {
+	if (nidx >= bvf->prm->nb_ins) {
 		RTE_BPF_LOG_LINE(ERR,
 			"%s: program boundary violation at pc: %u, next pc: %u",
 			__func__, get_node_idx(bvf, node), nidx);
@@ -1886,14 +1886,20 @@ get_prev_node(struct bpf_verifier *bvf, struct inst_node *node)
  * Control Flow Graph (CFG).
  * Information collected at this path would be used later
  * to determine is there any loops, and/or unreachable instructions.
+ * PREREQUISITE: there is at least one node.
  */
 static void
 dfs(struct bpf_verifier *bvf)
 {
 	struct inst_node *next, *node;
 
-	node = bvf->in;
-	while (node != NULL) {
+	RTE_ASSERT(bvf->nb_nodes != 0);
+	/*
+	 * Since there is at least one node, node with index 0 always exists;
+	 * it is our program entry point.
+	 */
+	node = &bvf->in[0];
+	do {
 
 		if (node->colour == WHITE)
 			set_node_colour(bvf, node, GREY);
@@ -1923,7 +1929,7 @@ dfs(struct bpf_verifier *bvf)
 			}
 		} else
 			node = NULL;
-	}
+	} while (node != NULL);
 }
 
 /*
@@ -2062,6 +2068,12 @@ validate(struct bpf_verifier *bvf)
 	if (rc != 0)
 		return rc;
 
+	if (bvf->nb_nodes == 0) {
+		RTE_BPF_LOG_LINE(ERR, "%s(%p) the program is empty",
+			__func__, bvf);
+		return -EINVAL;
+	}
+
 	dfs(bvf);
 
 	RTE_LOG(DEBUG, BPF, "%s(%p) stats:\n"
-- 
2.43.0

[PATCH 3/3] bpf: make add/subtract one program validate

[Marat Khalili]

Add tests loading simple BPF programs adding or subtracting one to its
argument and fix triggered signed integer overflow undefined behaviours:

    lib/bpf/bpf_validate.c:324:24: runtime error: signed integer
    overflow: 1 + 9223372036854775807 cannot be represented in type
    'long int'

    lib/bpf/bpf_validate.c:352:24: runtime error: signed integer
    overflow: -9223372036854775808 - 1 cannot be represented in type
    'long int'

As a minimal possible fix perform operation on unsigned integers where
overflow is well-defined, which was probably the original intent.

Signed-off-by: Marat Khalili <marat.khalili@huawei.com>
---
 app/test/test_bpf_simple.c | 58 ++++++++++++++++++++++++++++++++++++++
 lib/bpf/bpf_validate.c     |  8 +++---
 2 files changed, 62 insertions(+), 4 deletions(-)

diff --git a/app/test/test_bpf_simple.c b/app/test/test_bpf_simple.c
index 576a6ed029..d4c5bbdc6e 100644
--- a/app/test/test_bpf_simple.c
+++ b/app/test/test_bpf_simple.c
@@ -129,3 +129,61 @@ test_simple_minimal_working(void)
 
 REGISTER_FAST_TEST(bpf_simple_minimal_working_autotest, true, true,
 	test_simple_minimal_working);
+
+/*
+ * Try and load valid BPF program adding one to the argument.
+ */
+static int
+test_simple_add_one(void)
+{
+	static const struct ebpf_insn ins[] = {
+		{
+			/* Set return value to one. */
+			.code = (EBPF_ALU64 | EBPF_MOV | BPF_K),
+			.dst_reg = EBPF_REG_0,
+			.imm = 1,
+		},
+		{
+			/* Add program argument to the return value. */
+			.code = (EBPF_ALU64 | BPF_ADD | BPF_X),
+			.src_reg = EBPF_REG_1,
+			.dst_reg = EBPF_REG_0,
+		},
+		{
+			.code = (BPF_JMP | EBPF_EXIT),
+		},
+	};
+	return simple_bpf_load_test(RTE_DIM(ins), ins, 0);
+}
+
+REGISTER_FAST_TEST(bpf_simple_add_one_autotest, true, true,
+	test_simple_add_one);
+
+/*
+ * Try and load valid BPF program subtracting one from the argument.
+ */
+static int
+test_simple_subtract_one(void)
+{
+	static const struct ebpf_insn ins[] = {
+		{
+			/* Subtract one from the program argument. */
+			.code = (EBPF_ALU64 | BPF_SUB | BPF_K),
+			.dst_reg = EBPF_REG_1,
+			.imm = 1,
+		},
+		{
+			/* Set return value to the result. */
+			.code = (EBPF_ALU64 | EBPF_MOV | BPF_X),
+			.src_reg = EBPF_REG_1,
+			.dst_reg = EBPF_REG_0,
+		},
+		{
+			.code = (BPF_JMP | EBPF_EXIT),
+		},
+	};
+	return simple_bpf_load_test(RTE_DIM(ins), ins, 0);
+}
+
+REGISTER_FAST_TEST(bpf_simple_subtract_one_autotest, true, true,
+	test_simple_subtract_one);
diff --git a/lib/bpf/bpf_validate.c b/lib/bpf/bpf_validate.c
index 23444b3eaa..47ad6fef0f 100644
--- a/lib/bpf/bpf_validate.c
+++ b/lib/bpf/bpf_validate.c
@@ -243,8 +243,8 @@ eval_add(struct bpf_reg_val *rd, const struct bpf_reg_val *rs, uint64_t msk)
 
 	rv.u.min = (rd->u.min + rs->u.min) & msk;
 	rv.u.max = (rd->u.max + rs->u.max) & msk;
-	rv.s.min = (rd->s.min + rs->s.min) & msk;
-	rv.s.max = (rd->s.max + rs->s.max) & msk;
+	rv.s.min = ((uint64_t)rd->s.min + (uint64_t)rs->s.min) & msk;
+	rv.s.max = ((uint64_t)rd->s.max + (uint64_t)rs->s.max) & msk;
 
 	/*
 	 * if at least one of the operands is not constant,
@@ -272,8 +272,8 @@ eval_sub(struct bpf_reg_val *rd, const struct bpf_reg_val *rs, uint64_t msk)
 
 	rv.u.min = (rd->u.min - rs->u.max) & msk;
 	rv.u.max = (rd->u.max - rs->u.min) & msk;
-	rv.s.min = (rd->s.min - rs->s.max) & msk;
-	rv.s.max = (rd->s.max - rs->s.min) & msk;
+	rv.s.min = ((uint64_t)rd->s.min - (uint64_t)rs->s.max) & msk;
+	rv.s.max = ((uint64_t)rd->s.max - (uint64_t)rs->s.min) & msk;
 
 	/*
 	 * if at least one of the operands is not constant,
-- 
2.43.0

Re: [PATCH 2/3] bpf: disallow empty program

[Stephen Hemminger]

On Mon, 10 Nov 2025 15:30:45 +0000
Marat Khalili <marat.khalili@huawei.com> wrote:

> Add tests for some simple cases:
> * Program with no instructions;
> * Program with only EXIT instruction but no return value set;
> * Program with return value set but no EXIT instruction;
> * Minimal valid program with return value set and an EXIT instruction.
> 
> Fix found bugs:
> * a program with no instructions was accepted;
> * a program with no EXIT instruction read outside the buffer.
> 
> Signed-off-by: Marat Khalili <marat.khalili@huawei.com>

Can this just be folded into existing test_bpf file please.

RE: [PATCH 2/3] bpf: disallow empty program

[Marat Khalili]

> Can this just be folded into existing test_bpf file please.

Sure; any more issues? :)

Re: [PATCH 1/3] bpf: fix signed shift overflows in ARM JIT

[Jerin Jacob]

On Mon, Nov 10, 2025 at 9:01 PM Marat Khalili <marat.khalili@huawei.com> wrote:
>
> Left shifts of integer literals and bool values overwriting the sign bit
> were used multiple times in bpf_jit_arm64.c. E.g.:
>
>     insn = (!!is64) << 31;
>
> where is64 has type bool (double bang is a no-op here). The operand of
> left shift was promoted to type int, which when 32-bit wide cannot
> represent the result. Similarly literal integers have int type by
> default.  Sanitizer produced the following diagnostic during runtime
> (for various lines):
>
>     lib/bpf/bpf_jit_arm64.c:241:18: runtime error: left shift of 1 by 31
>     places cannot be represented in type 'int'

Wonder why none of the tests in app/test/test_bpf.c able to catch
this? The generated ARM opcode looks OK (otherwise tests wont pass).
Could you check what is missing in the app/test/test_bpf.c?

Also SHIFT_VAR32 needs goto common code.


>
> To fix the issue use RTE_BIT32 and similar macros instead.
>
> Signed-off-by: Marat Khalili <marat.khalili@huawei.com>
> ---
>  lib/bpf/bpf_jit_arm64.c | 188 +++++++++++++++++++++++-----------------
>  1 file changed, 107 insertions(+), 81 deletions(-)
>
> diff --git a/lib/bpf/bpf_jit_arm64.c b/lib/bpf/bpf_jit_arm64.c
> index 96b8cd2e03..5f43db0170 100644
> --- a/lib/bpf/bpf_jit_arm64.c
> +++ b/lib/bpf/bpf_jit_arm64.c
> @@ -28,7 +28,33 @@
>  #define A64_ZR                 31
>
>  #define check_imm(n, val) (((val) >= 0) ? !!((val) >> (n)) : !!((~val) >> (n)))
> -#define mask_imm(n, val) ((val) & ((1 << (n)) - 1))
> +#define mask_imm(n, val) ((val) & (RTE_BIT32(n) - 1))
> +
> +/**
> + * Get the uint32_t shifted value.
> + *
> + * Works similarly to RTE_SHIFT_VAL32 but accepts non-literal arguments.
> + * Performs identically to RTE_SHIFT_VAL32 with literal arguments.
> + *
> + * @param val
> + *   The value to be shifted, can be non-literal.
> + * @param nr
> + *   The shift number in range of 0 to (32 - width of val).
> + */
> +#define SHIFT_VAR32(val, nr) ((uint32_t)(val) << (nr))
> +
> +/**
> + * Get the uint64_t shifted value.
> + *
> + * Works similarly to RTE_SHIFT_VAL64 but accepts non-literal arguments.
> + * Performs identically to RTE_SHIFT_VAL64 with literal arguments.
> + *
> + * @param val
> + *   The value to be shifted, can be non-literal.
> + * @param nr
> + *   The shift number in range of 0 to (64 - width of val).
> + */
> +#define SHIFT_VAR64(val, nr) ((uint64_t)(val) << (nr))
>
>  struct ebpf_a64_map {
>         uint32_t off; /* eBPF to arm64 insn offset mapping for jump */
> @@ -238,12 +264,12 @@ emit_add_sub_imm(struct a64_jit_ctx *ctx, bool is64, bool sub, uint8_t rd,
>         uint32_t insn, imm;
>
>         imm = mask_imm(12, imm12);
> -       insn = (!!is64) << 31;
> -       insn |= (!!sub) << 30;
> +       insn = SHIFT_VAR32(is64, 31);
> +       insn |= SHIFT_VAR32(sub, 30);
>         insn |= 0x11000000;
>         insn |= rd;
> -       insn |= rn << 5;
> -       insn |= imm << 10;
> +       insn |= SHIFT_VAR32(rn, 5);
> +       insn |= SHIFT_VAR32(imm, 10);
>
>         emit_insn(ctx, insn,
>                   check_reg(rd) || check_reg(rn) || check_imm(12, imm12));
> @@ -279,16 +305,16 @@ emit_ls_pair_64(struct a64_jit_ctx *ctx, uint8_t rt, uint8_t rt2, uint8_t rn,
>  {
>         uint32_t insn;
>
> -       insn = (!!load) << 22;
> -       insn |= (!!pre_index) << 24;
> +       insn = SHIFT_VAR32(load, 22);
> +       insn |= SHIFT_VAR32(pre_index, 24);
>         insn |= 0xa8800000;
>         insn |= rt;
> -       insn |= rn << 5;
> -       insn |= rt2 << 10;
> +       insn |= SHIFT_VAR32(rn, 5);
> +       insn |= SHIFT_VAR32(rt2, 10);
>         if (push)
> -               insn |= 0x7e << 15; /* 0x7e means -2 with imm7 */
> +               insn |= SHIFT_VAR32(0x7e, 15); /* 0x7e means -2 with imm7 */
>         else
> -               insn |= 0x2 << 15;
> +               insn |= SHIFT_VAR32(0x2, 15);
>
>         emit_insn(ctx, insn, check_reg(rn) || check_reg(rt) || check_reg(rt2));
>
> @@ -317,11 +343,11 @@ mov_imm(struct a64_jit_ctx *ctx, bool is64, uint8_t rd, uint8_t type,
>  {
>         uint32_t insn;
>
> -       insn = (!!is64) << 31;
> -       insn |= type << 29;
> -       insn |= 0x25 << 23;
> -       insn |= (shift/16) << 21;
> -       insn |= imm16 << 5;
> +       insn = SHIFT_VAR32(is64, 31);
> +       insn |= SHIFT_VAR32(type, 29);
> +       insn |= SHIFT_VAR32(0x25, 23);
> +       insn |= SHIFT_VAR32(shift/16, 21);
> +       insn |= SHIFT_VAR32(imm16, 5);
>         insn |= rd;
>
>         emit_insn(ctx, insn, check_reg(rd) || check_mov_hw(is64, shift));
> @@ -334,7 +360,7 @@ emit_mov_imm32(struct a64_jit_ctx *ctx, bool is64, uint8_t rd, uint32_t val)
>         uint16_t lower = val & 0xffff;
>
>         /* Positive number */
> -       if ((val & 1UL << 31) == 0) {
> +       if ((val & RTE_BIT32(31)) == 0) {
>                 mov_imm(ctx, is64, rd, A64_MOVZ, lower, 0);
>                 if (upper)
>                         mov_imm(ctx, is64, rd, A64_MOVK, upper, 16);
> @@ -393,21 +419,21 @@ emit_ls(struct a64_jit_ctx *ctx, uint8_t sz, uint8_t rt, uint8_t rn, uint8_t rm,
>  {
>         uint32_t insn;
>
> -       insn = 0x1c1 << 21;
> +       insn = SHIFT_VAR32(0x1c1, 21);
>         if (load)
> -               insn |= 1 << 22;
> +               insn |= RTE_BIT32(22);
>         if (sz == BPF_B)
> -               insn |= 0 << 30;
> +               insn |= SHIFT_VAR32(0, 30);
>         else if (sz == BPF_H)
> -               insn |= 1 << 30;
> +               insn |= SHIFT_VAR32(1, 30);
>         else if (sz == BPF_W)
> -               insn |= 2 << 30;
> +               insn |= SHIFT_VAR32(2, 30);
>         else if (sz == EBPF_DW)
> -               insn |= 3 << 30;
> +               insn |= SHIFT_VAR32(3, 30);
>
> -       insn |= rm << 16;
> -       insn |= 0x1a << 10; /* LSL and S = 0 */
> -       insn |= rn << 5;
> +       insn |= SHIFT_VAR32(rm, 16);
> +       insn |= SHIFT_VAR32(0x1a, 10); /* LSL and S = 0 */
> +       insn |= SHIFT_VAR32(rn, 5);
>         insn |= rt;
>
>         emit_insn(ctx, insn, check_reg(rt) || check_reg(rn) || check_reg(rm) ||
> @@ -436,10 +462,10 @@ emit_add_sub(struct a64_jit_ctx *ctx, bool is64, uint8_t rd, uint8_t rn,
>  {
>         uint32_t insn;
>
> -       insn = (!!is64) << 31;
> -       insn |= op << 21; /* shift == 0 */
> -       insn |= rm << 16;
> -       insn |= rn << 5;
> +       insn = SHIFT_VAR32(is64, 31);
> +       insn |= SHIFT_VAR32(op, 21); /* shift == 0 */
> +       insn |= SHIFT_VAR32(rm, 16);
> +       insn |= SHIFT_VAR32(rn, 5);
>         insn |= rd;
>
>         emit_insn(ctx, insn, check_reg(rd) || check_reg(rm));
> @@ -468,11 +494,11 @@ emit_mul(struct a64_jit_ctx *ctx, bool is64, uint8_t rd, uint8_t rm)
>  {
>         uint32_t insn;
>
> -       insn = (!!is64) << 31;
> -       insn |= 0xd8 << 21;
> -       insn |= rm << 16;
> -       insn |= A64_ZR << 10;
> -       insn |= rd << 5;
> +       insn = SHIFT_VAR32(is64, 31);
> +       insn |= SHIFT_VAR32(0xd8, 21);
> +       insn |= SHIFT_VAR32(rm, 16);
> +       insn |= SHIFT_VAR32(A64_ZR, 10);
> +       insn |= SHIFT_VAR32(rd, 5);
>         insn |= rd;
>
>         emit_insn(ctx, insn, check_reg(rd) || check_reg(rm));
> @@ -489,11 +515,11 @@ emit_data_process_two_src(struct a64_jit_ctx *ctx, bool is64, uint8_t rd,
>  {
>         uint32_t insn;
>
> -       insn = (!!is64) << 31;
> -       insn |= 0xd6 << 21;
> -       insn |= rm << 16;
> -       insn |= op << 10;
> -       insn |= rn << 5;
> +       insn = SHIFT_VAR32(is64, 31);
> +       insn |= SHIFT_VAR32(0xd6, 21);
> +       insn |= SHIFT_VAR32(rm, 16);
> +       insn |= SHIFT_VAR32(op, 10);
> +       insn |= SHIFT_VAR32(rn, 5);
>         insn |= rd;
>
>         emit_insn(ctx, insn, check_reg(rd) || check_reg(rm));
> @@ -532,14 +558,14 @@ emit_bitfield(struct a64_jit_ctx *ctx, bool is64, uint8_t rd, uint8_t rn,
>  {
>         uint32_t insn;
>
> -       insn = (!!is64) << 31;
> +       insn = SHIFT_VAR32(is64, 31);
>         if (insn)
> -               insn |= 1 << 22; /* Set N bit when is64 is set */
> -       insn |= op << 29;
> -       insn |= 0x26 << 23;
> -       insn |= immr << 16;
> -       insn |= imms << 10;
> -       insn |= rn << 5;
> +               insn |= RTE_BIT32(22); /* Set N bit when is64 is set */
> +       insn |= SHIFT_VAR32(op, 29);
> +       insn |= SHIFT_VAR32(0x26, 23);
> +       insn |= SHIFT_VAR32(immr, 16);
> +       insn |= SHIFT_VAR32(imms, 10);
> +       insn |= SHIFT_VAR32(rn, 5);
>         insn |= rd;
>
>         emit_insn(ctx, insn, check_reg(rd) || check_reg(rn) ||
> @@ -578,11 +604,11 @@ emit_logical(struct a64_jit_ctx *ctx, bool is64, uint8_t rd,
>  {
>         uint32_t insn;
>
> -       insn = (!!is64) << 31;
> -       insn |= op << 29;
> -       insn |= 0x50 << 21;
> -       insn |= rm << 16;
> -       insn |= rd << 5;
> +       insn = SHIFT_VAR32(is64, 31);
> +       insn |= SHIFT_VAR32(op, 29);
> +       insn |= SHIFT_VAR32(0x50, 21);
> +       insn |= SHIFT_VAR32(rm, 16);
> +       insn |= SHIFT_VAR32(rd, 5);
>         insn |= rd;
>
>         emit_insn(ctx, insn, check_reg(rd) || check_reg(rm));
> @@ -612,12 +638,12 @@ emit_msub(struct a64_jit_ctx *ctx, bool is64, uint8_t rd, uint8_t rn,
>  {
>         uint32_t insn;
>
> -       insn = (!!is64) << 31;
> -       insn |= 0xd8 << 21;
> -       insn |= rm << 16;
> -       insn |= 0x1 << 15;
> -       insn |= ra << 10;
> -       insn |= rn << 5;
> +       insn = SHIFT_VAR32(is64, 31);
> +       insn |= SHIFT_VAR32(0xd8, 21);
> +       insn |= SHIFT_VAR32(rm, 16);
> +       insn |= SHIFT_VAR32(0x1, 15);
> +       insn |= SHIFT_VAR32(ra, 10);
> +       insn |= SHIFT_VAR32(rn, 5);
>         insn |= rd;
>
>         emit_insn(ctx, insn, check_reg(rd) || check_reg(rn) || check_reg(rm) ||
> @@ -638,7 +664,7 @@ emit_blr(struct a64_jit_ctx *ctx, uint8_t rn)
>         uint32_t insn;
>
>         insn = 0xd63f0000;
> -       insn |= rn << 5;
> +       insn |= SHIFT_VAR32(rn, 5);
>
>         emit_insn(ctx, insn, check_reg(rn));
>  }
> @@ -669,22 +695,22 @@ emit_rev(struct a64_jit_ctx *ctx, uint8_t rd, int32_t imm)
>         uint32_t insn;
>
>         insn = 0xdac00000;
> -       insn |= rd << 5;
> +       insn |= SHIFT_VAR32(rd, 5);
>         insn |= rd;
>
>         switch (imm) {
>         case 16:
> -               insn |= 1 << 10;
> +               insn |= SHIFT_VAR32(1, 10);
>                 emit_insn(ctx, insn, check_reg(rd));
>                 emit_zero_extend(ctx, rd, 16);
>                 break;
>         case 32:
> -               insn |= 2 << 10;
> +               insn |= SHIFT_VAR32(2, 10);
>                 emit_insn(ctx, insn, check_reg(rd));
>                 /* Upper 32 bits already cleared */
>                 break;
>         case 64:
> -               insn |= 3 << 10;
> +               insn |= SHIFT_VAR32(3, 10);
>                 emit_insn(ctx, insn, check_reg(rd));
>                 break;
>         default:
> @@ -933,9 +959,9 @@ emit_cbnz(struct a64_jit_ctx *ctx, bool is64, uint8_t rt, int32_t imm19)
>         uint32_t insn, imm;
>
>         imm = mask_imm(19, imm19);
> -       insn = (!!is64) << 31;
> -       insn |= 0x35 << 24;
> -       insn |= imm << 5;
> +       insn = SHIFT_VAR32(is64, 31);
> +       insn |= SHIFT_VAR32(0x35, 24);
> +       insn |= SHIFT_VAR32(imm, 5);
>         insn |= rt;
>
>         emit_insn(ctx, insn, check_reg(rt) || check_imm(19, imm19));
> @@ -947,7 +973,7 @@ emit_b(struct a64_jit_ctx *ctx, int32_t imm26)
>         uint32_t insn, imm;
>
>         imm = mask_imm(26, imm26);
> -       insn = 0x5 << 26;
> +       insn = SHIFT_VAR32(0x5, 26);
>         insn |= imm;
>
>         emit_insn(ctx, insn, check_imm(26, imm26));
> @@ -971,9 +997,9 @@ emit_stadd(struct a64_jit_ctx *ctx, bool is64, uint8_t rs, uint8_t rn)
>         uint32_t insn;
>
>         insn = 0xb820001f;
> -       insn |= (!!is64) << 30;
> -       insn |= rs << 16;
> -       insn |= rn << 5;
> +       insn |= SHIFT_VAR32(is64, 30);
> +       insn |= SHIFT_VAR32(rs, 16);
> +       insn |= SHIFT_VAR32(rn, 5);
>
>         emit_insn(ctx, insn, check_reg(rs) || check_reg(rn));
>  }
> @@ -984,8 +1010,8 @@ emit_ldxr(struct a64_jit_ctx *ctx, bool is64, uint8_t rt, uint8_t rn)
>         uint32_t insn;
>
>         insn = 0x885f7c00;
> -       insn |= (!!is64) << 30;
> -       insn |= rn << 5;
> +       insn |= SHIFT_VAR32(is64, 30);
> +       insn |= SHIFT_VAR32(rn, 5);
>         insn |= rt;
>
>         emit_insn(ctx, insn, check_reg(rt) || check_reg(rn));
> @@ -998,9 +1024,9 @@ emit_stxr(struct a64_jit_ctx *ctx, bool is64, uint8_t rs, uint8_t rt,
>         uint32_t insn;
>
>         insn = 0x88007c00;
> -       insn |= (!!is64) << 30;
> -       insn |= rs << 16;
> -       insn |= rn << 5;
> +       insn |= SHIFT_VAR32(is64, 30);
> +       insn |= SHIFT_VAR32(rs, 16);
> +       insn |= SHIFT_VAR32(rn, 5);
>         insn |= rt;
>
>         emit_insn(ctx, insn, check_reg(rs) || check_reg(rt) || check_reg(rn));
> @@ -1051,9 +1077,9 @@ emit_cmp_tst(struct a64_jit_ctx *ctx, bool is64, uint8_t rn, uint8_t rm,
>         uint32_t insn;
>
>         insn = opc;
> -       insn |= (!!is64) << 31;
> -       insn |= rm << 16;
> -       insn |= rn << 5;
> +       insn |= SHIFT_VAR32(is64, 31);
> +       insn |= SHIFT_VAR32(rm, 16);
> +       insn |= SHIFT_VAR32(rn, 5);
>
>         emit_insn(ctx, insn, check_reg(rn) || check_reg(rm));
>  }
> @@ -1076,8 +1102,8 @@ emit_b_cond(struct a64_jit_ctx *ctx, uint8_t cond, int32_t imm19)
>         uint32_t insn, imm;
>
>         imm = mask_imm(19, imm19);
> -       insn = 0x15 << 26;
> -       insn |= imm << 5;
> +       insn = SHIFT_VAR32(0x15, 26);
> +       insn |= SHIFT_VAR32(imm, 5);
>         insn |= cond;
>
>         emit_insn(ctx, insn, check_cond(cond) || check_imm(19, imm19));
> @@ -1301,7 +1327,7 @@ emit(struct a64_jit_ctx *ctx, struct rte_bpf *bpf)
>                         break;
>                 /* dst = imm64 */
>                 case (BPF_LD | BPF_IMM | EBPF_DW):
> -                       u64 = ((uint64_t)ins[1].imm << 32) | (uint32_t)imm;
> +                       u64 = SHIFT_VAR64(ins[1].imm, 32) | (uint32_t)imm;
>                         emit_mov_imm(ctx, 1, dst, u64);
>                         i++;
>                         break;
> --
> 2.43.0
>

RE: [PATCH 1/3] bpf: fix signed shift overflows in ARM JIT

[Morten Brørup]

> From: Jerin Jacob [mailto:jerinjacobk@gmail.com]
> Sent: Tuesday, 11 November 2025 07.25
> 
> On Mon, Nov 10, 2025 at 9:01 PM Marat Khalili
> <marat.khalili@huawei.com> wrote:
> >
> > Left shifts of integer literals and bool values overwriting the sign
> bit
> > were used multiple times in bpf_jit_arm64.c. E.g.:
> >
> >     insn = (!!is64) << 31;
> >
> > where is64 has type bool (double bang is a no-op here). The operand
> of
> > left shift was promoted to type int, which when 32-bit wide cannot
> > represent the result. Similarly literal integers have int type by
> > default.  Sanitizer produced the following diagnostic during runtime
> > (for various lines):
> >
> >     lib/bpf/bpf_jit_arm64.c:241:18: runtime error: left shift of 1 by
> 31
> >     places cannot be represented in type 'int'
> 
> Wonder why none of the tests in app/test/test_bpf.c able to catch
> this? The generated ARM opcode looks OK (otherwise tests wont pass).
> Could you check what is missing in the app/test/test_bpf.c?
> 
> Also SHIFT_VAR32 needs goto common code.
> 
> 
> >
> > To fix the issue use RTE_BIT32 and similar macros instead.
> >
> > Signed-off-by: Marat Khalili <marat.khalili@huawei.com>
> > ---
> >  lib/bpf/bpf_jit_arm64.c | 188 +++++++++++++++++++++++---------------
> --
> >  1 file changed, 107 insertions(+), 81 deletions(-)
> >
> > diff --git a/lib/bpf/bpf_jit_arm64.c b/lib/bpf/bpf_jit_arm64.c
> > index 96b8cd2e03..5f43db0170 100644
> > --- a/lib/bpf/bpf_jit_arm64.c
> > +++ b/lib/bpf/bpf_jit_arm64.c
> > @@ -28,7 +28,33 @@
> >  #define A64_ZR                 31
> >
> >  #define check_imm(n, val) (((val) >= 0) ? !!((val) >> (n)) :
> !!((~val) >> (n)))
> > -#define mask_imm(n, val) ((val) & ((1 << (n)) - 1))
> > +#define mask_imm(n, val) ((val) & (RTE_BIT32(n) - 1))
> > +
> > +/**
> > + * Get the uint32_t shifted value.
> > + *
> > + * Works similarly to RTE_SHIFT_VAL32 but accepts non-literal
> arguments.
> > + * Performs identically to RTE_SHIFT_VAL32 with literal arguments.
> > + *
> > + * @param val
> > + *   The value to be shifted, can be non-literal.
> > + * @param nr
> > + *   The shift number in range of 0 to (32 - width of val).
> > + */
> > +#define SHIFT_VAR32(val, nr) ((uint32_t)(val) << (nr))
> > +
> > +/**
> > + * Get the uint64_t shifted value.
> > + *
> > + * Works similarly to RTE_SHIFT_VAL64 but accepts non-literal
> arguments.
> > + * Performs identically to RTE_SHIFT_VAL64 with literal arguments.
> > + *
> > + * @param val
> > + *   The value to be shifted, can be non-literal.
> > + * @param nr
> > + *   The shift number in range of 0 to (64 - width of val).
> > + */
> > +#define SHIFT_VAR64(val, nr) ((uint64_t)(val) << (nr))

Better replace the RTE_SHIFT_VAL32/VAL64 macros with these, to support both literal and non-literal arguments.

RE: [PATCH 1/3] bpf: fix signed shift overflows in ARM JIT

[Marat Khalili]

> On Mon, Nov 10, 2025 at 9:01 PM Marat Khalili <marat.khalili@huawei.com> wrote:
> >
> > Left shifts of integer literals and bool values overwriting the sign bit
> > were used multiple times in bpf_jit_arm64.c. E.g.:
> >
> >     insn = (!!is64) << 31;
> >
> > where is64 has type bool (double bang is a no-op here). The operand of
> > left shift was promoted to type int, which when 32-bit wide cannot
> > represent the result. Similarly literal integers have int type by
> > default.  Sanitizer produced the following diagnostic during runtime
> > (for various lines):
> >
> >     lib/bpf/bpf_jit_arm64.c:241:18: runtime error: left shift of 1 by 31
> >     places cannot be represented in type 'int'
> 
> Wonder why none of the tests in app/test/test_bpf.c able to catch
> this? The generated ARM opcode looks OK (otherwise tests wont pass).
> Could you check what is missing in the app/test/test_bpf.c?

That tests do trigger it.

RTE>>bpf_autotest
../../src/lib/bpf/bpf_jit_arm64.c:320:18: runtime error: left shift of 1 by 31 places cannot be represented in type 'int'
    #0 0x22bf59c in mov_imm ../../src/lib/bpf/bpf_jit_arm64.c:320
    #1 0x22bfae0 in emit_mov_imm ../../src/lib/bpf/bpf_jit_arm64.c:378
    #2 0x22c3bfc in emit ../../src/lib/bpf/bpf_jit_arm64.c:1321
    #3 0x22c4314 in __rte_bpf_jit_arm64 ../../src/lib/bpf/bpf_jit_arm64.c:1414
    #4 0x22775c0 in __rte_bpf_jit ../../src/lib/bpf/bpf.c:44
    #5 0x228fbdc in rte_bpf_load ../../src/lib/bpf/bpf_load.c:115
    #6 0x6312f8 in run_test ../../src/app/test/test_bpf.c:3225
    #7 0x631848 in test_bpf ../../src/app/test/test_bpf.c:3269
[...]

RTE>>bpf_convert_autotest
../../src/lib/bpf/bpf_jit_arm64.c:241:18: runtime error: left shift of 1 by 31 places cannot be represented in type 'int'
    #0 0x22bf04c in emit_add_sub_imm ../../src/lib/bpf/bpf_jit_arm64.c:241
    #1 0x22bf270 in emit_mov ../../src/lib/bpf/bpf_jit_arm64.c:267
    #2 0x22bf2a4 in emit_mov_64 ../../src/lib/bpf/bpf_jit_arm64.c:273
    #3 0x22c1a68 in emit_prologue_no_call ../../src/lib/bpf/bpf_jit_arm64.c:884
    #4 0x22c1c9c in emit_prologue ../../src/lib/bpf/bpf_jit_arm64.c:904
    #5 0x22c327c in emit ../../src/lib/bpf/bpf_jit_arm64.c:1136
    #6 0x22c4314 in __rte_bpf_jit_arm64 ../../src/lib/bpf/bpf_jit_arm64.c:1414
    #7 0x22775c0 in __rte_bpf_jit ../../src/lib/bpf/bpf.c:44
    #8 0x228fbdc in rte_bpf_load ../../src/lib/bpf/bpf_load.c:115
    #9 0x631e7c in test_bpf_match ../../src/app/test/test_bpf.c:3328
    #10 0x632ce8 in test_bpf_filter_sanity ../../src/app/test/test_bpf.c:3376
    #11 0x6331c4 in test_bpf_convert ../../src/app/test/test_bpf.c:3487
[...]

Re: [PATCH 1/3] bpf: fix signed shift overflows in ARM JIT

[Jerin Jacob]

On Tue, Nov 11, 2025 at 3:40 PM Marat Khalili <marat.khalili@huawei.com> wrote:
>
> > On Mon, Nov 10, 2025 at 9:01 PM Marat Khalili <marat.khalili@huawei.com> wrote:
> > >
> > > Left shifts of integer literals and bool values overwriting the sign bit
> > > were used multiple times in bpf_jit_arm64.c. E.g.:
> > >
> > >     insn = (!!is64) << 31;
> > >
> > > where is64 has type bool (double bang is a no-op here). The operand of
> > > left shift was promoted to type int, which when 32-bit wide cannot
> > > represent the result. Similarly literal integers have int type by
> > > default.  Sanitizer produced the following diagnostic during runtime
> > > (for various lines):
> > >
> > >     lib/bpf/bpf_jit_arm64.c:241:18: runtime error: left shift of 1 by 31
> > >     places cannot be represented in type 'int'
> >
> > Wonder why none of the tests in app/test/test_bpf.c able to catch
> > this? The generated ARM opcode looks OK (otherwise tests wont pass).
> > Could you check what is missing in the app/test/test_bpf.c?
>
> That tests do trigger it.
>
> RTE>>bpf_autotest
> ../../src/lib/bpf/bpf_jit_arm64.c:320:18: runtime error: left shift of 1 by 31 places cannot be represented in type 'int'
>     #0 0x22bf59c in mov_imm ../../src/lib/bpf/bpf_jit_arm64.c:320
>     #1 0x22bfae0 in emit_mov_imm ../../src/lib/bpf/bpf_jit_arm64.c:378
>     #2 0x22c3bfc in emit ../../src/lib/bpf/bpf_jit_arm64.c:1321
>     #3 0x22c4314 in __rte_bpf_jit_arm64 ../../src/lib/bpf/bpf_jit_arm64.c:1414
>     #4 0x22775c0 in __rte_bpf_jit ../../src/lib/bpf/bpf.c:44
>     #5 0x228fbdc in rte_bpf_load ../../src/lib/bpf/bpf_load.c:115
>     #6 0x6312f8 in run_test ../../src/app/test/test_bpf.c:3225
>     #7 0x631848 in test_bpf ../../src/app/test/test_bpf.c:3269
> [...]
>
> RTE>>bpf_convert_autotest
> ../../src/lib/bpf/bpf_jit_arm64.c:241:18: runtime error: left shift of 1 by 31 places cannot be represented in type 'int'
>     #0 0x22bf04c in emit_add_sub_imm ../../src/lib/bpf/bpf_jit_arm64.c:241
>     #1 0x22bf270 in emit_mov ../../src/lib/bpf/bpf_jit_arm64.c:267
>     #2 0x22bf2a4 in emit_mov_64 ../../src/lib/bpf/bpf_jit_arm64.c:273
>     #3 0x22c1a68 in emit_prologue_no_call ../../src/lib/bpf/bpf_jit_arm64.c:884
>     #4 0x22c1c9c in emit_prologue ../../src/lib/bpf/bpf_jit_arm64.c:904
>     #5 0x22c327c in emit ../../src/lib/bpf/bpf_jit_arm64.c:1136
>     #6 0x22c4314 in __rte_bpf_jit_arm64 ../../src/lib/bpf/bpf_jit_arm64.c:1414
>     #7 0x22775c0 in __rte_bpf_jit ../../src/lib/bpf/bpf.c:44
>     #8 0x228fbdc in rte_bpf_load ../../src/lib/bpf/bpf_load.c:115
>     #9 0x631e7c in test_bpf_match ../../src/app/test/test_bpf.c:3328
>     #10 0x632ce8 in test_bpf_filter_sanity ../../src/app/test/test_bpf.c:3376
>     #11 0x6331c4 in test_bpf_convert ../../src/app/test/test_bpf.c:3487
> [...]

Re: [PATCH 1/3] bpf: fix signed shift overflows in ARM JIT

[Jerin Jacob]

On Tue, Nov 11, 2025 at 3:40 PM Marat Khalili <marat.khalili@huawei.com> wrote:
>
> > On Mon, Nov 10, 2025 at 9:01 PM Marat Khalili <marat.khalili@huawei.com> wrote:
> > >
> > > Left shifts of integer literals and bool values overwriting the sign bit
> > > were used multiple times in bpf_jit_arm64.c. E.g.:
> > >
> > >     insn = (!!is64) << 31;
> > >
> > > where is64 has type bool (double bang is a no-op here). The operand of
> > > left shift was promoted to type int, which when 32-bit wide cannot
> > > represent the result. Similarly literal integers have int type by
> > > default.  Sanitizer produced the following diagnostic during runtime
> > > (for various lines):
> > >
> > >     lib/bpf/bpf_jit_arm64.c:241:18: runtime error: left shift of 1 by 31
> > >     places cannot be represented in type 'int'
> >
> > Wonder why none of the tests in app/test/test_bpf.c able to catch
> > this? The generated ARM opcode looks OK (otherwise tests wont pass).
> > Could you check what is missing in the app/test/test_bpf.c?
>
> That tests do trigger it.
>
> RTE>>bpf_autotest
> ../../src/lib/bpf/bpf_jit_arm64.c:320:18: runtime error: left shift of 1 by 31 places cannot be represented in type 'int'

This is when ASAN is enabled. Right? Is it really generating BAD
opcode(i.e emiter generating bad OPcode) and test fails?


>     #0 0x22bf59c in mov_imm ../../src/lib/bpf/bpf_jit_arm64.c:320
>     #1 0x22bfae0 in emit_mov_imm ../../src/lib/bpf/bpf_jit_arm64.c:378
>     #2 0x22c3bfc in emit ../../src/lib/bpf/bpf_jit_arm64.c:1321
>     #3 0x22c4314 in __rte_bpf_jit_arm64 ../../src/lib/bpf/bpf_jit_arm64.c:1414
>     #4 0x22775c0 in __rte_bpf_jit ../../src/lib/bpf/bpf.c:44
>     #5 0x228fbdc in rte_bpf_load ../../src/lib/bpf/bpf_load.c:115
>     #6 0x6312f8 in run_test ../../src/app/test/test_bpf.c:3225
>     #7 0x631848 in test_bpf ../../src/app/test/test_bpf.c:3269
> [...]
>
> RTE>>bpf_convert_autotest
> ../../src/lib/bpf/bpf_jit_arm64.c:241:18: runtime error: left shift of 1 by 31 places cannot be represented in type 'int'
>     #0 0x22bf04c in emit_add_sub_imm ../../src/lib/bpf/bpf_jit_arm64.c:241
>     #1 0x22bf270 in emit_mov ../../src/lib/bpf/bpf_jit_arm64.c:267
>     #2 0x22bf2a4 in emit_mov_64 ../../src/lib/bpf/bpf_jit_arm64.c:273
>     #3 0x22c1a68 in emit_prologue_no_call ../../src/lib/bpf/bpf_jit_arm64.c:884
>     #4 0x22c1c9c in emit_prologue ../../src/lib/bpf/bpf_jit_arm64.c:904
>     #5 0x22c327c in emit ../../src/lib/bpf/bpf_jit_arm64.c:1136
>     #6 0x22c4314 in __rte_bpf_jit_arm64 ../../src/lib/bpf/bpf_jit_arm64.c:1414
>     #7 0x22775c0 in __rte_bpf_jit ../../src/lib/bpf/bpf.c:44
>     #8 0x228fbdc in rte_bpf_load ../../src/lib/bpf/bpf_load.c:115
>     #9 0x631e7c in test_bpf_match ../../src/app/test/test_bpf.c:3328
>     #10 0x632ce8 in test_bpf_filter_sanity ../../src/app/test/test_bpf.c:3376
>     #11 0x6331c4 in test_bpf_convert ../../src/app/test/test_bpf.c:3487
> [...]

RE: [PATCH 1/3] bpf: fix signed shift overflows in ARM JIT

[Marat Khalili]

> -----Original Message-----
> From: Jerin Jacob <jerinjacobk@gmail.com>
> Sent: Tuesday 11 November 2025 16:31
> To: Marat Khalili <marat.khalili@huawei.com>
> Cc: Konstantin Ananyev <konstantin.ananyev@huawei.com>; Stephen Hemminger <stephen@networkplumber.org>;
> dev@dpdk.org
> Subject: Re: [PATCH 1/3] bpf: fix signed shift overflows in ARM JIT
> 
> On Tue, Nov 11, 2025 at 3:40 PM Marat Khalili <marat.khalili@huawei.com> wrote:
> >
> > > On Mon, Nov 10, 2025 at 9:01 PM Marat Khalili <marat.khalili@huawei.com> wrote:
> > > >
> > > > Left shifts of integer literals and bool values overwriting the sign bit
> > > > were used multiple times in bpf_jit_arm64.c. E.g.:
> > > >
> > > >     insn = (!!is64) << 31;
> > > >
> > > > where is64 has type bool (double bang is a no-op here). The operand of
> > > > left shift was promoted to type int, which when 32-bit wide cannot
> > > > represent the result. Similarly literal integers have int type by
> > > > default.  Sanitizer produced the following diagnostic during runtime
> > > > (for various lines):
> > > >
> > > >     lib/bpf/bpf_jit_arm64.c:241:18: runtime error: left shift of 1 by 31
> > > >     places cannot be represented in type 'int'
> > >
> > > Wonder why none of the tests in app/test/test_bpf.c able to catch
> > > this? The generated ARM opcode looks OK (otherwise tests wont pass).
> > > Could you check what is missing in the app/test/test_bpf.c?
> >
> > That tests do trigger it.
> >
> > RTE>>bpf_autotest
> > ../../src/lib/bpf/bpf_jit_arm64.c:320:18: runtime error: left shift of 1 by 31 places cannot be
> represented in type 'int'
> 
> This is when ASAN is enabled. Right?

More likely UBSAN, but yes.

> Is it really generating BAD
> opcode(i.e emiter generating bad OPcode) and test fails?

If allowed to finish the test does not fail.
However, I have not compared DPDK or JIT compiled code with and without changes.

Re: [PATCH 1/3] bpf: fix signed shift overflows in ARM JIT

[Jerin Jacob]

On Tue, Nov 11, 2025 at 10:09 PM Marat Khalili <marat.khalili@huawei.com> wrote:
>
> > -----Original Message-----
> > From: Jerin Jacob <jerinjacobk@gmail.com>
> > Sent: Tuesday 11 November 2025 16:31
> > To: Marat Khalili <marat.khalili@huawei.com>
> > Cc: Konstantin Ananyev <konstantin.ananyev@huawei.com>; Stephen Hemminger <stephen@networkplumber.org>;
> > dev@dpdk.org
> > Subject: Re: [PATCH 1/3] bpf: fix signed shift overflows in ARM JIT
> >
> > On Tue, Nov 11, 2025 at 3:40 PM Marat Khalili <marat.khalili@huawei.com> wrote:
> > >
> > > > On Mon, Nov 10, 2025 at 9:01 PM Marat Khalili <marat.khalili@huawei.com> wrote:
> > > > >
> > > > > Left shifts of integer literals and bool values overwriting the sign bit
> > > > > were used multiple times in bpf_jit_arm64.c. E.g.:
> > > > >
> > > > >     insn = (!!is64) << 31;
> > > > >
> > > > > where is64 has type bool (double bang is a no-op here). The operand of
> > > > > left shift was promoted to type int, which when 32-bit wide cannot
> > > > > represent the result. Similarly literal integers have int type by
> > > > > default.  Sanitizer produced the following diagnostic during runtime
> > > > > (for various lines):
> > > > >
> > > > >     lib/bpf/bpf_jit_arm64.c:241:18: runtime error: left shift of 1 by 31
> > > > >     places cannot be represented in type 'int'
> > > >
> > > > Wonder why none of the tests in app/test/test_bpf.c able to catch
> > > > this? The generated ARM opcode looks OK (otherwise tests wont pass).
> > > > Could you check what is missing in the app/test/test_bpf.c?
> > >
> > > That tests do trigger it.
> > >
> > > RTE>>bpf_autotest
> > > ../../src/lib/bpf/bpf_jit_arm64.c:320:18: runtime error: left shift of 1 by 31 places cannot be
> > represented in type 'int'
> >
> > This is when ASAN is enabled. Right?
>
> More likely UBSAN, but yes.
>
> > Is it really generating BAD
> > opcode(i.e emiter generating bad OPcode) and test fails?
>
> If allowed to finish the test does not fail.

Does that mean, it is a false positive? Right?

Anyway, There is no harm in the new patch, Change the patch to use
SHIFT macros from EAL.


> However, I have not compared DPDK or JIT compiled code with and without changes.

RE: [PATCH 1/3] bpf: fix signed shift overflows in ARM JIT

[Marat Khalili]

> > > > That tests do trigger it.
> > > >
> > > > RTE>>bpf_autotest
> > > > ../../src/lib/bpf/bpf_jit_arm64.c:320:18: runtime error: left shift of 1 by 31 places cannot be
> > > represented in type 'int'
> > >
> > > This is when ASAN is enabled. Right?
> >
> > More likely UBSAN, but yes.
> >
> > > Is it really generating BAD
> > > opcode(i.e emiter generating bad OPcode) and test fails?
> >
> > If allowed to finish the test does not fail.
> 
> Does that mean, it is a false positive? Right?

It depends on who you ask. Certain compilers with certain compilation options
tend to delete execution branches that cause UB, and the test may not be
detailed enough to notice this.

> Anyway, There is no harm in the new patch, Change the patch to use
> SHIFT macros from EAL.

Ok, will do. It is probably too late for the current release anyway.

RE: [PATCH 2/3] bpf: disallow empty program

[Konstantin Ananyev]

> Add tests for some simple cases:
> * Program with no instructions;
> * Program with only EXIT instruction but no return value set;
> * Program with return value set but no EXIT instruction;
> * Minimal valid program with return value set and an EXIT instruction.
> 
> Fix found bugs:
> * a program with no instructions was accepted;
> * a program with no EXIT instruction read outside the buffer.
> 
> Signed-off-by: Marat Khalili <marat.khalili@huawei.com>
> ---
>  app/test/meson.build       |   1 +
>  app/test/test_bpf_simple.c | 131 +++++++++++++++++++++++++++++++++++++
>  lib/bpf/bpf_load.c         |   2 +-
>  lib/bpf/bpf_validate.c     |  20 ++++--
>  4 files changed, 149 insertions(+), 5 deletions(-)
>  create mode 100644 app/test/test_bpf_simple.c
> 
> diff --git a/app/test/meson.build b/app/test/meson.build
> index 8df8d3edd1..9d48431ba0 100644
> --- a/app/test/meson.build
> +++ b/app/test/meson.build
> @@ -35,6 +35,7 @@ source_file_deps = {
>      'test_bitset.c': [],
>      'test_bitratestats.c': ['metrics', 'bitratestats', 'ethdev'] +
> sample_packet_forward_deps,
>      'test_bpf.c': ['bpf', 'net'],
> +    'test_bpf_simple.c': ['bpf'],
>      'test_byteorder.c': [],
>      'test_cfgfile.c': ['cfgfile'],
>      'test_cksum.c': ['net'],
> diff --git a/app/test/test_bpf_simple.c b/app/test/test_bpf_simple.c
> new file mode 100644
> index 0000000000..576a6ed029
> --- /dev/null
> +++ b/app/test/test_bpf_simple.c
> @@ -0,0 +1,131 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2025 Huawei Technologies Co., Ltd
> + */
> +
> +#include "test.h"
> +
> +#include <rte_bpf.h>
> +#include <rte_errno.h>
> +
> +/* Tests of most simple BPF programs (no instructions, one instruction etc.) */
> +
> +/*
> + * Try to load a simple bpf program from the instructions array.
> + *
> + * When `expected_errno` is zero, expect it to load successfully.
> + * When `expected_errno` is non-zero, expect it to fail with this `rte_errno`.
> + *
> + * @param nb_ins
> + *   Number of instructions in the `ins` array.
> + * @param ins
> + *   BPF instructions array.
> + * @param expected_errno
> + *   Expected result.
> + * @return
> + *   TEST_SUCCESS on success, error code on failure.
> + */
> +static int
> +simple_bpf_load_test(uint32_t nb_ins, const struct ebpf_insn *ins,
> +	int expected_errno)
> +{
> +	const struct rte_bpf_prm prm = {
> +		.ins = ins,
> +		.nb_ins = nb_ins,
> +		.prog_arg = {
> +			.type = RTE_BPF_ARG_RAW,
> +			.size = sizeof(uint64_t),
> +		},
> +	};
> +
> +	struct rte_bpf *const bpf = rte_bpf_load(&prm);
> +	const int actual_errno = rte_errno;
> +	rte_bpf_destroy(bpf);
> +
> +	if (expected_errno != 0) {
> +		RTE_TEST_ASSERT_EQUAL(bpf, NULL,
> +			"expect rte_bpf_load() == NULL");
> +		RTE_TEST_ASSERT_EQUAL(actual_errno, expected_errno,
> +			"expect rte_errno == %d, found %d",
> +			expected_errno, actual_errno);
> +	} else
> +		RTE_TEST_ASSERT_NOT_EQUAL(bpf, NULL,
> +			"expect rte_bpf_load() != NULL");
> +
> +	return TEST_SUCCESS;
> +}
> +
> +/*
> + * Try and load completely empty BPF program.
> + * Should fail because there is no EXIT (and also return value is undefined).
> + */
> +static int
> +test_simple_no_instructions(void)
> +{
> +	static const struct ebpf_insn ins[] = {};
> +	return simple_bpf_load_test(RTE_DIM(ins), ins, EINVAL);
> +}
> +
> +REGISTER_FAST_TEST(bpf_simple_no_instructions_autotest, true, true,
> +	test_simple_no_instructions);
> +
> +/*
> + * Try and load a BPF program comprising single EXIT instruction.
> + * Should fail because the return value is undefined.
> + */
> +static int
> +test_simple_exit_only(void)
> +{
> +	static const struct ebpf_insn ins[] = {
> +		{
> +			.code = (BPF_JMP | EBPF_EXIT),
> +		},
> +	};
> +	return simple_bpf_load_test(RTE_DIM(ins), ins, EINVAL);
> +}
> +
> +REGISTER_FAST_TEST(bpf_simple_exit_only_autotest, true, true,
> +	test_simple_exit_only);
> +
> +/*
> + * Try and load a BPF program with no EXIT instruction.
> + * Should fail because of this.
> + */
> +static int
> +test_simple_no_exit(void)
> +{
> +	static const struct ebpf_insn ins[] = {
> +		{
> +			/* Set return value to the program argument. */
> +			.code = (EBPF_ALU64 | EBPF_MOV | BPF_X),
> +			.src_reg = EBPF_REG_1,
> +			.dst_reg = EBPF_REG_0,
> +		},
> +	};
> +	return simple_bpf_load_test(RTE_DIM(ins), ins, EINVAL);
> +}
> +
> +REGISTER_FAST_TEST(bpf_simple_no_exit_autotest, true, true,
> +	test_simple_no_exit);
> +
> +/*
> + * Try and load smallest possible valid BPF program.
> + */
> +static int
> +test_simple_minimal_working(void)
> +{
> +	static const struct ebpf_insn ins[] = {
> +		{
> +			/* Set return value to the program argument. */
> +			.code = (EBPF_ALU64 | EBPF_MOV | BPF_X),
> +			.src_reg = EBPF_REG_1,
> +			.dst_reg = EBPF_REG_0,
> +		},
> +		{
> +			.code = (BPF_JMP | EBPF_EXIT),
> +		},
> +	};
> +	return simple_bpf_load_test(RTE_DIM(ins), ins, 0);
> +}
> +
> +REGISTER_FAST_TEST(bpf_simple_minimal_working_autotest, true, true,
> +	test_simple_minimal_working);
> diff --git a/lib/bpf/bpf_load.c b/lib/bpf/bpf_load.c
> index 556e613762..6983c026af 100644
> --- a/lib/bpf/bpf_load.c
> +++ b/lib/bpf/bpf_load.c
> @@ -88,7 +88,7 @@ rte_bpf_load(const struct rte_bpf_prm *prm)
>  	int32_t rc;
>  	uint32_t i;
> 
> -	if (prm == NULL || prm->ins == NULL ||
> +	if (prm == NULL || prm->ins == NULL || prm->nb_ins == 0 ||
>  			(prm->nb_xsym != 0 && prm->xsym == NULL)) {
>  		rte_errno = EINVAL;
>  		return NULL;
> diff --git a/lib/bpf/bpf_validate.c b/lib/bpf/bpf_validate.c
> index 4f47d6dc7b..23444b3eaa 100644
> --- a/lib/bpf/bpf_validate.c
> +++ b/lib/bpf/bpf_validate.c
> @@ -1827,7 +1827,7 @@ add_edge(struct bpf_verifier *bvf, struct inst_node
> *node, uint32_t nidx)
>  {
>  	uint32_t ne;
> 
> -	if (nidx > bvf->prm->nb_ins) {
> +	if (nidx >= bvf->prm->nb_ins) {
>  		RTE_BPF_LOG_LINE(ERR,
>  			"%s: program boundary violation at pc: %u, next pc: %u",
>  			__func__, get_node_idx(bvf, node), nidx);
> @@ -1886,14 +1886,20 @@ get_prev_node(struct bpf_verifier *bvf, struct
> inst_node *node)
>   * Control Flow Graph (CFG).
>   * Information collected at this path would be used later
>   * to determine is there any loops, and/or unreachable instructions.
> + * PREREQUISITE: there is at least one node.
>   */
>  static void
>  dfs(struct bpf_verifier *bvf)
>  {
>  	struct inst_node *next, *node;
> 
> -	node = bvf->in;
> -	while (node != NULL) {
> +	RTE_ASSERT(bvf->nb_nodes != 0);
> +	/*
> +	 * Since there is at least one node, node with index 0 always exists;
> +	 * it is our program entry point.
> +	 */
> +	node = &bvf->in[0];
> +	do {
> 
>  		if (node->colour == WHITE)
>  			set_node_colour(bvf, node, GREY);
> @@ -1923,7 +1929,7 @@ dfs(struct bpf_verifier *bvf)
>  			}
>  		} else
>  			node = NULL;
> -	}
> +	} while (node != NULL);
>  }
> 
>  /*
> @@ -2062,6 +2068,12 @@ validate(struct bpf_verifier *bvf)
>  	if (rc != 0)
>  		return rc;
> 
> +	if (bvf->nb_nodes == 0) {
> +		RTE_BPF_LOG_LINE(ERR, "%s(%p) the program is empty",
> +			__func__, bvf);
> +		return -EINVAL;
> +	}
> +
>  	dfs(bvf);
> 
>  	RTE_LOG(DEBUG, BPF, "%s(%p) stats:\n"
> --

Acked-by: Konstantin Ananyev <konstantin.ananyev@huawei.com>

> 2.43.0

RE: [PATCH 3/3] bpf: make add/subtract one program validate

[Konstantin Ananyev]

> Add tests loading simple BPF programs adding or subtracting one to its
> argument and fix triggered signed integer overflow undefined behaviours:
> 
>     lib/bpf/bpf_validate.c:324:24: runtime error: signed integer
>     overflow: 1 + 9223372036854775807 cannot be represented in type
>     'long int'
> 
>     lib/bpf/bpf_validate.c:352:24: runtime error: signed integer
>     overflow: -9223372036854775808 - 1 cannot be represented in type
>     'long int'
> 
> As a minimal possible fix perform operation on unsigned integers where
> overflow is well-defined, which was probably the original intent.
> 
> Signed-off-by: Marat Khalili <marat.khalili@huawei.com>
> ---
>  app/test/test_bpf_simple.c | 58 ++++++++++++++++++++++++++++++++++++++
>  lib/bpf/bpf_validate.c     |  8 +++---
>  2 files changed, 62 insertions(+), 4 deletions(-)
> 
> diff --git a/app/test/test_bpf_simple.c b/app/test/test_bpf_simple.c
> index 576a6ed029..d4c5bbdc6e 100644
> --- a/app/test/test_bpf_simple.c
> +++ b/app/test/test_bpf_simple.c
> @@ -129,3 +129,61 @@ test_simple_minimal_working(void)
> 
>  REGISTER_FAST_TEST(bpf_simple_minimal_working_autotest, true, true,
>  	test_simple_minimal_working);
> +
> +/*
> + * Try and load valid BPF program adding one to the argument.
> + */
> +static int
> +test_simple_add_one(void)
> +{
> +	static const struct ebpf_insn ins[] = {
> +		{
> +			/* Set return value to one. */
> +			.code = (EBPF_ALU64 | EBPF_MOV | BPF_K),
> +			.dst_reg = EBPF_REG_0,
> +			.imm = 1,
> +		},
> +		{
> +			/* Add program argument to the return value. */
> +			.code = (EBPF_ALU64 | BPF_ADD | BPF_X),
> +			.src_reg = EBPF_REG_1,
> +			.dst_reg = EBPF_REG_0,
> +		},
> +		{
> +			.code = (BPF_JMP | EBPF_EXIT),
> +		},
> +	};
> +	return simple_bpf_load_test(RTE_DIM(ins), ins, 0);
> +}
> +
> +REGISTER_FAST_TEST(bpf_simple_add_one_autotest, true, true,
> +	test_simple_add_one);
> +
> +/*
> + * Try and load valid BPF program subtracting one from the argument.
> + */
> +static int
> +test_simple_subtract_one(void)
> +{
> +	static const struct ebpf_insn ins[] = {
> +		{
> +			/* Subtract one from the program argument. */
> +			.code = (EBPF_ALU64 | BPF_SUB | BPF_K),
> +			.dst_reg = EBPF_REG_1,
> +			.imm = 1,
> +		},
> +		{
> +			/* Set return value to the result. */
> +			.code = (EBPF_ALU64 | EBPF_MOV | BPF_X),
> +			.src_reg = EBPF_REG_1,
> +			.dst_reg = EBPF_REG_0,
> +		},
> +		{
> +			.code = (BPF_JMP | EBPF_EXIT),
> +		},
> +	};
> +	return simple_bpf_load_test(RTE_DIM(ins), ins, 0);
> +}
> +
> +REGISTER_FAST_TEST(bpf_simple_subtract_one_autotest, true, true,
> +	test_simple_subtract_one);
> diff --git a/lib/bpf/bpf_validate.c b/lib/bpf/bpf_validate.c
> index 23444b3eaa..47ad6fef0f 100644
> --- a/lib/bpf/bpf_validate.c
> +++ b/lib/bpf/bpf_validate.c
> @@ -243,8 +243,8 @@ eval_add(struct bpf_reg_val *rd, const struct bpf_reg_val
> *rs, uint64_t msk)
> 
>  	rv.u.min = (rd->u.min + rs->u.min) & msk;
>  	rv.u.max = (rd->u.max + rs->u.max) & msk;
> -	rv.s.min = (rd->s.min + rs->s.min) & msk;
> -	rv.s.max = (rd->s.max + rs->s.max) & msk;
> +	rv.s.min = ((uint64_t)rd->s.min + (uint64_t)rs->s.min) & msk;
> +	rv.s.max = ((uint64_t)rd->s.max + (uint64_t)rs->s.max) & msk;
> 
>  	/*
>  	 * if at least one of the operands is not constant,
> @@ -272,8 +272,8 @@ eval_sub(struct bpf_reg_val *rd, const struct bpf_reg_val
> *rs, uint64_t msk)
> 
>  	rv.u.min = (rd->u.min - rs->u.max) & msk;
>  	rv.u.max = (rd->u.max - rs->u.min) & msk;
> -	rv.s.min = (rd->s.min - rs->s.max) & msk;
> -	rv.s.max = (rd->s.max - rs->s.min) & msk;
> +	rv.s.min = ((uint64_t)rd->s.min - (uint64_t)rs->s.max) & msk;
> +	rv.s.max = ((uint64_t)rd->s.max - (uint64_t)rs->s.min) & msk;
> 
>  	/*
>  	 * if at least one of the operands is not constant,
> --

Acked-by: Konstantin Ananyev <konstantin.ananyev@huawei.com>

> 2.43.0

[PATCH v2 0/5] bpf: simple tests and fixes

[Marat Khalili]

Add simple unit-style tests for rte_bpf_load, and fix some minor
discovered bugs.

v2:
* Moved new tests back into test_bpf.c
* Changed library macros RTE_SHIFT_VALxx to handle variable first
  arguments instead of introducing new macros.
* Added another test and fix, for conditional jump as first instruction.

Marat Khalili (5):
  eal: variable first arguments of RTE_SHIFT_VALxx
  bpf: fix signed shift overflows in ARM JIT
  bpf: disallow empty program
  bpf: make add/subtract one program validate
  bpf: fix BPF validation w/ conditional jump first

 app/test/test_bpf.c          | 254 +++++++++++++++++++++++++++++++++++
 lib/bpf/bpf_jit_arm64.c      | 162 +++++++++++-----------
 lib/bpf/bpf_load.c           |   2 +-
 lib/bpf/bpf_validate.c       |  48 ++++---
 lib/eal/include/rte_bitops.h |   4 +-
 5 files changed, 364 insertions(+), 106 deletions(-)

-- 
2.43.0

[PATCH v2 1/5] eal: variable first arguments of RTE_SHIFT_VALxx

[Marat Khalili]

Macros RTE_SHIFT_VAL32 and RTE_SHIFT_VAL64 were implemented by applying
UINT32_C or UINT64_C correspondingly to its first argument. As a
consequence first argument had to be a constant. Replace UINT32_C and
UINT64_C with casts to uint32_t and uint64_t to allow these arguments be
variable. For constants the result should be the same.

(Yes, technically UINT64_C promotes to uint_least64_t, not uint64_t, but
I think most users of RTE_SHIFT_VAL64 expect the result to be uint64_t.)

Signed-off-by: Marat Khalili <marat.khalili@huawei.com>
---
 lib/eal/include/rte_bitops.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/eal/include/rte_bitops.h b/lib/eal/include/rte_bitops.h
index 2d1b9d281c..aa6ac73abb 100644
--- a/lib/eal/include/rte_bitops.h
+++ b/lib/eal/include/rte_bitops.h
@@ -51,7 +51,7 @@ extern "C" {
  * @param nr
  *   The shift number in range of 0 to (32 - width of val).
  */
-#define RTE_SHIFT_VAL32(val, nr) (UINT32_C(val) << (nr))
+#define RTE_SHIFT_VAL32(val, nr) ((uint32_t)(val) << (nr))
 
 /**
  * Get the uint64_t shifted value.
@@ -61,7 +61,7 @@ extern "C" {
  * @param nr
  *   The shift number in range of 0 to (64 - width of val).
  */
-#define RTE_SHIFT_VAL64(val, nr) (UINT64_C(val) << (nr))
+#define RTE_SHIFT_VAL64(val, nr) ((uint64_t)(val) << (nr))
 
 /**
  * Generate a contiguous 32-bit mask
-- 
2.43.0

[PATCH v2 2/5] bpf: fix signed shift overflows in ARM JIT

[Marat Khalili]

Left shifts of integer literals and bool values overwriting the sign bit
were used multiple times in bpf_jit_arm64.c. E.g.:

    insn = (!!is64) << 31;

where is64 has type bool (double bang is a no-op here). The operand of
left shift was promoted to type int, which when 32-bit wide cannot
represent the result. Similarly literal integers have int type by
default.  Sanitizer produced the following diagnostic during runtime
(for various lines):

    lib/bpf/bpf_jit_arm64.c:241:18: runtime error: left shift of 1 by 31
    places cannot be represented in type 'int'

To fix the issue use RTE_BIT32 and similar macros instead.

Signed-off-by: Marat Khalili <marat.khalili@huawei.com>
---
 lib/bpf/bpf_jit_arm64.c | 162 ++++++++++++++++++++--------------------
 1 file changed, 81 insertions(+), 81 deletions(-)

diff --git a/lib/bpf/bpf_jit_arm64.c b/lib/bpf/bpf_jit_arm64.c
index 96b8cd2e03..76df1e4ba1 100644
--- a/lib/bpf/bpf_jit_arm64.c
+++ b/lib/bpf/bpf_jit_arm64.c
@@ -28,7 +28,7 @@
 #define A64_ZR			31
 
 #define check_imm(n, val) (((val) >= 0) ? !!((val) >> (n)) : !!((~val) >> (n)))
-#define mask_imm(n, val) ((val) & ((1 << (n)) - 1))
+#define mask_imm(n, val) ((val) & (RTE_BIT32(n) - 1))
 
 struct ebpf_a64_map {
 	uint32_t off; /* eBPF to arm64 insn offset mapping for jump */
@@ -238,12 +238,12 @@ emit_add_sub_imm(struct a64_jit_ctx *ctx, bool is64, bool sub, uint8_t rd,
 	uint32_t insn, imm;
 
 	imm = mask_imm(12, imm12);
-	insn = (!!is64) << 31;
-	insn |= (!!sub) << 30;
+	insn = RTE_SHIFT_VAL32(is64, 31);
+	insn |= RTE_SHIFT_VAL32(sub, 30);
 	insn |= 0x11000000;
 	insn |= rd;
-	insn |= rn << 5;
-	insn |= imm << 10;
+	insn |= RTE_SHIFT_VAL32(rn, 5);
+	insn |= RTE_SHIFT_VAL32(imm, 10);
 
 	emit_insn(ctx, insn,
 		  check_reg(rd) || check_reg(rn) || check_imm(12, imm12));
@@ -279,16 +279,16 @@ emit_ls_pair_64(struct a64_jit_ctx *ctx, uint8_t rt, uint8_t rt2, uint8_t rn,
 {
 	uint32_t insn;
 
-	insn = (!!load) << 22;
-	insn |= (!!pre_index) << 24;
+	insn = RTE_SHIFT_VAL32(load, 22);
+	insn |= RTE_SHIFT_VAL32(pre_index, 24);
 	insn |= 0xa8800000;
 	insn |= rt;
-	insn |= rn << 5;
-	insn |= rt2 << 10;
+	insn |= RTE_SHIFT_VAL32(rn, 5);
+	insn |= RTE_SHIFT_VAL32(rt2, 10);
 	if (push)
-		insn |= 0x7e << 15; /* 0x7e means -2 with imm7 */
+		insn |= RTE_SHIFT_VAL32(0x7e, 15); /* 0x7e means -2 with imm7 */
 	else
-		insn |= 0x2 << 15;
+		insn |= RTE_SHIFT_VAL32(0x2, 15);
 
 	emit_insn(ctx, insn, check_reg(rn) || check_reg(rt) || check_reg(rt2));
 
@@ -317,11 +317,11 @@ mov_imm(struct a64_jit_ctx *ctx, bool is64, uint8_t rd, uint8_t type,
 {
 	uint32_t insn;
 
-	insn = (!!is64) << 31;
-	insn |= type << 29;
-	insn |= 0x25 << 23;
-	insn |= (shift/16) << 21;
-	insn |= imm16 << 5;
+	insn = RTE_SHIFT_VAL32(is64, 31);
+	insn |= RTE_SHIFT_VAL32(type, 29);
+	insn |= RTE_SHIFT_VAL32(0x25, 23);
+	insn |= RTE_SHIFT_VAL32(shift/16, 21);
+	insn |= RTE_SHIFT_VAL32(imm16, 5);
 	insn |= rd;
 
 	emit_insn(ctx, insn, check_reg(rd) || check_mov_hw(is64, shift));
@@ -334,7 +334,7 @@ emit_mov_imm32(struct a64_jit_ctx *ctx, bool is64, uint8_t rd, uint32_t val)
 	uint16_t lower = val & 0xffff;
 
 	/* Positive number */
-	if ((val & 1UL << 31) == 0) {
+	if ((val & RTE_BIT32(31)) == 0) {
 		mov_imm(ctx, is64, rd, A64_MOVZ, lower, 0);
 		if (upper)
 			mov_imm(ctx, is64, rd, A64_MOVK, upper, 16);
@@ -393,21 +393,21 @@ emit_ls(struct a64_jit_ctx *ctx, uint8_t sz, uint8_t rt, uint8_t rn, uint8_t rm,
 {
 	uint32_t insn;
 
-	insn = 0x1c1 << 21;
+	insn = RTE_SHIFT_VAL32(0x1c1, 21);
 	if (load)
-		insn |= 1 << 22;
+		insn |= RTE_BIT32(22);
 	if (sz == BPF_B)
-		insn |= 0 << 30;
+		insn |= RTE_SHIFT_VAL32(0, 30);
 	else if (sz == BPF_H)
-		insn |= 1 << 30;
+		insn |= RTE_SHIFT_VAL32(1, 30);
 	else if (sz == BPF_W)
-		insn |= 2 << 30;
+		insn |= RTE_SHIFT_VAL32(2, 30);
 	else if (sz == EBPF_DW)
-		insn |= 3 << 30;
+		insn |= RTE_SHIFT_VAL32(3, 30);
 
-	insn |= rm << 16;
-	insn |= 0x1a << 10; /* LSL and S = 0 */
-	insn |= rn << 5;
+	insn |= RTE_SHIFT_VAL32(rm, 16);
+	insn |= RTE_SHIFT_VAL32(0x1a, 10); /* LSL and S = 0 */
+	insn |= RTE_SHIFT_VAL32(rn, 5);
 	insn |= rt;
 
 	emit_insn(ctx, insn, check_reg(rt) || check_reg(rn) || check_reg(rm) ||
@@ -436,10 +436,10 @@ emit_add_sub(struct a64_jit_ctx *ctx, bool is64, uint8_t rd, uint8_t rn,
 {
 	uint32_t insn;
 
-	insn = (!!is64) << 31;
-	insn |= op << 21; /* shift == 0 */
-	insn |= rm << 16;
-	insn |= rn << 5;
+	insn = RTE_SHIFT_VAL32(is64, 31);
+	insn |= RTE_SHIFT_VAL32(op, 21); /* shift == 0 */
+	insn |= RTE_SHIFT_VAL32(rm, 16);
+	insn |= RTE_SHIFT_VAL32(rn, 5);
 	insn |= rd;
 
 	emit_insn(ctx, insn, check_reg(rd) || check_reg(rm));
@@ -468,11 +468,11 @@ emit_mul(struct a64_jit_ctx *ctx, bool is64, uint8_t rd, uint8_t rm)
 {
 	uint32_t insn;
 
-	insn = (!!is64) << 31;
-	insn |= 0xd8 << 21;
-	insn |= rm << 16;
-	insn |= A64_ZR << 10;
-	insn |= rd << 5;
+	insn = RTE_SHIFT_VAL32(is64, 31);
+	insn |= RTE_SHIFT_VAL32(0xd8, 21);
+	insn |= RTE_SHIFT_VAL32(rm, 16);
+	insn |= RTE_SHIFT_VAL32(A64_ZR, 10);
+	insn |= RTE_SHIFT_VAL32(rd, 5);
 	insn |= rd;
 
 	emit_insn(ctx, insn, check_reg(rd) || check_reg(rm));
@@ -489,11 +489,11 @@ emit_data_process_two_src(struct a64_jit_ctx *ctx, bool is64, uint8_t rd,
 {
 	uint32_t insn;
 
-	insn = (!!is64) << 31;
-	insn |= 0xd6 << 21;
-	insn |= rm << 16;
-	insn |= op << 10;
-	insn |= rn << 5;
+	insn = RTE_SHIFT_VAL32(is64, 31);
+	insn |= RTE_SHIFT_VAL32(0xd6, 21);
+	insn |= RTE_SHIFT_VAL32(rm, 16);
+	insn |= RTE_SHIFT_VAL32(op, 10);
+	insn |= RTE_SHIFT_VAL32(rn, 5);
 	insn |= rd;
 
 	emit_insn(ctx, insn, check_reg(rd) || check_reg(rm));
@@ -532,14 +532,14 @@ emit_bitfield(struct a64_jit_ctx *ctx, bool is64, uint8_t rd, uint8_t rn,
 {
 	uint32_t insn;
 
-	insn = (!!is64) << 31;
+	insn = RTE_SHIFT_VAL32(is64, 31);
 	if (insn)
-		insn |= 1 << 22; /* Set N bit when is64 is set */
-	insn |= op << 29;
-	insn |= 0x26 << 23;
-	insn |= immr << 16;
-	insn |= imms << 10;
-	insn |= rn << 5;
+		insn |= RTE_BIT32(22); /* Set N bit when is64 is set */
+	insn |= RTE_SHIFT_VAL32(op, 29);
+	insn |= RTE_SHIFT_VAL32(0x26, 23);
+	insn |= RTE_SHIFT_VAL32(immr, 16);
+	insn |= RTE_SHIFT_VAL32(imms, 10);
+	insn |= RTE_SHIFT_VAL32(rn, 5);
 	insn |= rd;
 
 	emit_insn(ctx, insn, check_reg(rd) || check_reg(rn) ||
@@ -578,11 +578,11 @@ emit_logical(struct a64_jit_ctx *ctx, bool is64, uint8_t rd,
 {
 	uint32_t insn;
 
-	insn = (!!is64) << 31;
-	insn |= op << 29;
-	insn |= 0x50 << 21;
-	insn |= rm << 16;
-	insn |= rd << 5;
+	insn = RTE_SHIFT_VAL32(is64, 31);
+	insn |= RTE_SHIFT_VAL32(op, 29);
+	insn |= RTE_SHIFT_VAL32(0x50, 21);
+	insn |= RTE_SHIFT_VAL32(rm, 16);
+	insn |= RTE_SHIFT_VAL32(rd, 5);
 	insn |= rd;
 
 	emit_insn(ctx, insn, check_reg(rd) || check_reg(rm));
@@ -612,12 +612,12 @@ emit_msub(struct a64_jit_ctx *ctx, bool is64, uint8_t rd, uint8_t rn,
 {
 	uint32_t insn;
 
-	insn = (!!is64) << 31;
-	insn |= 0xd8 << 21;
-	insn |= rm << 16;
-	insn |= 0x1 << 15;
-	insn |= ra << 10;
-	insn |= rn << 5;
+	insn = RTE_SHIFT_VAL32(is64, 31);
+	insn |= RTE_SHIFT_VAL32(0xd8, 21);
+	insn |= RTE_SHIFT_VAL32(rm, 16);
+	insn |= RTE_SHIFT_VAL32(0x1, 15);
+	insn |= RTE_SHIFT_VAL32(ra, 10);
+	insn |= RTE_SHIFT_VAL32(rn, 5);
 	insn |= rd;
 
 	emit_insn(ctx, insn, check_reg(rd) || check_reg(rn) || check_reg(rm) ||
@@ -638,7 +638,7 @@ emit_blr(struct a64_jit_ctx *ctx, uint8_t rn)
 	uint32_t insn;
 
 	insn = 0xd63f0000;
-	insn |= rn << 5;
+	insn |= RTE_SHIFT_VAL32(rn, 5);
 
 	emit_insn(ctx, insn, check_reg(rn));
 }
@@ -669,22 +669,22 @@ emit_rev(struct a64_jit_ctx *ctx, uint8_t rd, int32_t imm)
 	uint32_t insn;
 
 	insn = 0xdac00000;
-	insn |= rd << 5;
+	insn |= RTE_SHIFT_VAL32(rd, 5);
 	insn |= rd;
 
 	switch (imm) {
 	case 16:
-		insn |= 1 << 10;
+		insn |= RTE_SHIFT_VAL32(1, 10);
 		emit_insn(ctx, insn, check_reg(rd));
 		emit_zero_extend(ctx, rd, 16);
 		break;
 	case 32:
-		insn |= 2 << 10;
+		insn |= RTE_SHIFT_VAL32(2, 10);
 		emit_insn(ctx, insn, check_reg(rd));
 		/* Upper 32 bits already cleared */
 		break;
 	case 64:
-		insn |= 3 << 10;
+		insn |= RTE_SHIFT_VAL32(3, 10);
 		emit_insn(ctx, insn, check_reg(rd));
 		break;
 	default:
@@ -933,9 +933,9 @@ emit_cbnz(struct a64_jit_ctx *ctx, bool is64, uint8_t rt, int32_t imm19)
 	uint32_t insn, imm;
 
 	imm = mask_imm(19, imm19);
-	insn = (!!is64) << 31;
-	insn |= 0x35 << 24;
-	insn |= imm << 5;
+	insn = RTE_SHIFT_VAL32(is64, 31);
+	insn |= RTE_SHIFT_VAL32(0x35, 24);
+	insn |= RTE_SHIFT_VAL32(imm, 5);
 	insn |= rt;
 
 	emit_insn(ctx, insn, check_reg(rt) || check_imm(19, imm19));
@@ -947,7 +947,7 @@ emit_b(struct a64_jit_ctx *ctx, int32_t imm26)
 	uint32_t insn, imm;
 
 	imm = mask_imm(26, imm26);
-	insn = 0x5 << 26;
+	insn = RTE_SHIFT_VAL32(0x5, 26);
 	insn |= imm;
 
 	emit_insn(ctx, insn, check_imm(26, imm26));
@@ -971,9 +971,9 @@ emit_stadd(struct a64_jit_ctx *ctx, bool is64, uint8_t rs, uint8_t rn)
 	uint32_t insn;
 
 	insn = 0xb820001f;
-	insn |= (!!is64) << 30;
-	insn |= rs << 16;
-	insn |= rn << 5;
+	insn |= RTE_SHIFT_VAL32(is64, 30);
+	insn |= RTE_SHIFT_VAL32(rs, 16);
+	insn |= RTE_SHIFT_VAL32(rn, 5);
 
 	emit_insn(ctx, insn, check_reg(rs) || check_reg(rn));
 }
@@ -984,8 +984,8 @@ emit_ldxr(struct a64_jit_ctx *ctx, bool is64, uint8_t rt, uint8_t rn)
 	uint32_t insn;
 
 	insn = 0x885f7c00;
-	insn |= (!!is64) << 30;
-	insn |= rn << 5;
+	insn |= RTE_SHIFT_VAL32(is64, 30);
+	insn |= RTE_SHIFT_VAL32(rn, 5);
 	insn |= rt;
 
 	emit_insn(ctx, insn, check_reg(rt) || check_reg(rn));
@@ -998,9 +998,9 @@ emit_stxr(struct a64_jit_ctx *ctx, bool is64, uint8_t rs, uint8_t rt,
 	uint32_t insn;
 
 	insn = 0x88007c00;
-	insn |= (!!is64) << 30;
-	insn |= rs << 16;
-	insn |= rn << 5;
+	insn |= RTE_SHIFT_VAL32(is64, 30);
+	insn |= RTE_SHIFT_VAL32(rs, 16);
+	insn |= RTE_SHIFT_VAL32(rn, 5);
 	insn |= rt;
 
 	emit_insn(ctx, insn, check_reg(rs) || check_reg(rt) || check_reg(rn));
@@ -1051,9 +1051,9 @@ emit_cmp_tst(struct a64_jit_ctx *ctx, bool is64, uint8_t rn, uint8_t rm,
 	uint32_t insn;
 
 	insn = opc;
-	insn |= (!!is64) << 31;
-	insn |= rm << 16;
-	insn |= rn << 5;
+	insn |= RTE_SHIFT_VAL32(is64, 31);
+	insn |= RTE_SHIFT_VAL32(rm, 16);
+	insn |= RTE_SHIFT_VAL32(rn, 5);
 
 	emit_insn(ctx, insn, check_reg(rn) || check_reg(rm));
 }
@@ -1076,8 +1076,8 @@ emit_b_cond(struct a64_jit_ctx *ctx, uint8_t cond, int32_t imm19)
 	uint32_t insn, imm;
 
 	imm = mask_imm(19, imm19);
-	insn = 0x15 << 26;
-	insn |= imm << 5;
+	insn = RTE_SHIFT_VAL32(0x15, 26);
+	insn |= RTE_SHIFT_VAL32(imm, 5);
 	insn |= cond;
 
 	emit_insn(ctx, insn, check_cond(cond) || check_imm(19, imm19));
@@ -1301,7 +1301,7 @@ emit(struct a64_jit_ctx *ctx, struct rte_bpf *bpf)
 			break;
 		/* dst = imm64 */
 		case (BPF_LD | BPF_IMM | EBPF_DW):
-			u64 = ((uint64_t)ins[1].imm << 32) | (uint32_t)imm;
+			u64 = RTE_SHIFT_VAL64(ins[1].imm, 32) | (uint32_t)imm;
 			emit_mov_imm(ctx, 1, dst, u64);
 			i++;
 			break;
-- 
2.43.0

[PATCH v2 3/5] bpf: disallow empty program

[Marat Khalili]

Add tests for some simple cases:
* Program with no instructions;
* Program with only EXIT instruction but no return value set;
* Program with return value set but no EXIT instruction;
* Minimal valid program with return value set and an EXIT instruction.

Fix found bugs:
* a program with no instructions was accepted;
* a program with no EXIT instruction read outside the buffer.

Signed-off-by: Marat Khalili <marat.khalili@huawei.com>
Acked-by: Konstantin Ananyev <konstantin.ananyev@huawei.com>
---
 app/test/test_bpf.c    | 118 +++++++++++++++++++++++++++++++++++++++++
 lib/bpf/bpf_load.c     |   2 +-
 lib/bpf/bpf_validate.c |  20 +++++--
 3 files changed, 135 insertions(+), 5 deletions(-)

diff --git a/app/test/test_bpf.c b/app/test/test_bpf.c
index b7c94ba1c7..6ecc49efff 100644
--- a/app/test/test_bpf.c
+++ b/app/test/test_bpf.c
@@ -34,6 +34,124 @@ test_bpf(void)
 #include <rte_ip.h>
 
 
+/* Tests of most simple BPF programs (no instructions, one instruction etc.) */
+
+/*
+ * Try to load a simple bpf program from the instructions array.
+ *
+ * When `expected_errno` is zero, expect it to load successfully.
+ * When `expected_errno` is non-zero, expect it to fail with this `rte_errno`.
+ *
+ * @param nb_ins
+ *   Number of instructions in the `ins` array.
+ * @param ins
+ *   BPF instructions array.
+ * @param expected_errno
+ *   Expected result.
+ * @return
+ *   TEST_SUCCESS on success, error code on failure.
+ */
+static int
+bpf_load_test(uint32_t nb_ins, const struct ebpf_insn *ins, int expected_errno)
+{
+	const struct rte_bpf_prm prm = {
+		.ins = ins,
+		.nb_ins = nb_ins,
+		.prog_arg = {
+			.type = RTE_BPF_ARG_RAW,
+			.size = sizeof(uint64_t),
+		},
+	};
+
+	struct rte_bpf *const bpf = rte_bpf_load(&prm);
+	const int actual_errno = rte_errno;
+	rte_bpf_destroy(bpf);
+
+	if (expected_errno != 0) {
+		RTE_TEST_ASSERT_EQUAL(bpf, NULL,
+			"expect rte_bpf_load() == NULL");
+		RTE_TEST_ASSERT_EQUAL(actual_errno, expected_errno,
+			"expect rte_errno == %d, found %d",
+			expected_errno, actual_errno);
+	} else
+		RTE_TEST_ASSERT_NOT_EQUAL(bpf, NULL,
+			"expect rte_bpf_load() != NULL");
+
+	return TEST_SUCCESS;
+}
+
+/*
+ * Try and load completely empty BPF program.
+ * Should fail because there is no EXIT (and also return value is undefined).
+ */
+static int
+test_no_instructions(void)
+{
+	static const struct ebpf_insn ins[] = {};
+	return bpf_load_test(RTE_DIM(ins), ins, EINVAL);
+}
+
+REGISTER_FAST_TEST(bpf_no_instructions_autotest, true, true, test_no_instructions);
+
+/*
+ * Try and load a BPF program comprising single EXIT instruction.
+ * Should fail because the return value is undefined.
+ */
+static int
+test_exit_only(void)
+{
+	static const struct ebpf_insn ins[] = {
+		{
+			.code = (BPF_JMP | EBPF_EXIT),
+		},
+	};
+	return bpf_load_test(RTE_DIM(ins), ins, EINVAL);
+}
+
+REGISTER_FAST_TEST(bpf_exit_only_autotest, true, true, test_exit_only);
+
+/*
+ * Try and load a BPF program with no EXIT instruction.
+ * Should fail because of this.
+ */
+static int
+test_no_exit(void)
+{
+	static const struct ebpf_insn ins[] = {
+		{
+			/* Set return value to the program argument. */
+			.code = (EBPF_ALU64 | EBPF_MOV | BPF_X),
+			.src_reg = EBPF_REG_1,
+			.dst_reg = EBPF_REG_0,
+		},
+	};
+	return bpf_load_test(RTE_DIM(ins), ins, EINVAL);
+}
+
+REGISTER_FAST_TEST(bpf_no_exit_autotest, true, true, test_no_exit);
+
+/*
+ * Try and load smallest possible valid BPF program.
+ */
+static int
+test_minimal_working(void)
+{
+	static const struct ebpf_insn ins[] = {
+		{
+			/* Set return value to the program argument. */
+			.code = (EBPF_ALU64 | EBPF_MOV | BPF_X),
+			.src_reg = EBPF_REG_1,
+			.dst_reg = EBPF_REG_0,
+		},
+		{
+			.code = (BPF_JMP | EBPF_EXIT),
+		},
+	};
+	return bpf_load_test(RTE_DIM(ins), ins, 0);
+}
+
+REGISTER_FAST_TEST(bpf_minimal_working_autotest, true, true, test_minimal_working);
+
 /*
  * Basic functional tests for librte_bpf.
  * The main procedure - load eBPF program, execute it and
diff --git a/lib/bpf/bpf_load.c b/lib/bpf/bpf_load.c
index 556e613762..6983c026af 100644
--- a/lib/bpf/bpf_load.c
+++ b/lib/bpf/bpf_load.c
@@ -88,7 +88,7 @@ rte_bpf_load(const struct rte_bpf_prm *prm)
 	int32_t rc;
 	uint32_t i;
 
-	if (prm == NULL || prm->ins == NULL ||
+	if (prm == NULL || prm->ins == NULL || prm->nb_ins == 0 ||
 			(prm->nb_xsym != 0 && prm->xsym == NULL)) {
 		rte_errno = EINVAL;
 		return NULL;
diff --git a/lib/bpf/bpf_validate.c b/lib/bpf/bpf_validate.c
index 4f47d6dc7b..23444b3eaa 100644
--- a/lib/bpf/bpf_validate.c
+++ b/lib/bpf/bpf_validate.c
@@ -1827,7 +1827,7 @@ add_edge(struct bpf_verifier *bvf, struct inst_node *node, uint32_t nidx)
 {
 	uint32_t ne;
 
-	if (nidx > bvf->prm->nb_ins) {
+	if (nidx >= bvf->prm->nb_ins) {
 		RTE_BPF_LOG_LINE(ERR,
 			"%s: program boundary violation at pc: %u, next pc: %u",
 			__func__, get_node_idx(bvf, node), nidx);
@@ -1886,14 +1886,20 @@ get_prev_node(struct bpf_verifier *bvf, struct inst_node *node)
  * Control Flow Graph (CFG).
  * Information collected at this path would be used later
  * to determine is there any loops, and/or unreachable instructions.
+ * PREREQUISITE: there is at least one node.
  */
 static void
 dfs(struct bpf_verifier *bvf)
 {
 	struct inst_node *next, *node;
 
-	node = bvf->in;
-	while (node != NULL) {
+	RTE_ASSERT(bvf->nb_nodes != 0);
+	/*
+	 * Since there is at least one node, node with index 0 always exists;
+	 * it is our program entry point.
+	 */
+	node = &bvf->in[0];
+	do {
 
 		if (node->colour == WHITE)
 			set_node_colour(bvf, node, GREY);
@@ -1923,7 +1929,7 @@ dfs(struct bpf_verifier *bvf)
 			}
 		} else
 			node = NULL;
-	}
+	} while (node != NULL);
 }
 
 /*
@@ -2062,6 +2068,12 @@ validate(struct bpf_verifier *bvf)
 	if (rc != 0)
 		return rc;
 
+	if (bvf->nb_nodes == 0) {
+		RTE_BPF_LOG_LINE(ERR, "%s(%p) the program is empty",
+			__func__, bvf);
+		return -EINVAL;
+	}
+
 	dfs(bvf);
 
 	RTE_LOG(DEBUG, BPF, "%s(%p) stats:\n"
-- 
2.43.0

[PATCH v2 4/5] bpf: make add/subtract one program validate

[Marat Khalili]

Add tests loading simple BPF programs adding or subtracting one to its
argument and fix triggered signed integer overflow undefined behaviours:

    lib/bpf/bpf_validate.c:324:24: runtime error: signed integer
    overflow: 1 + 9223372036854775807 cannot be represented in type
    'long int'

    lib/bpf/bpf_validate.c:352:24: runtime error: signed integer
    overflow: -9223372036854775808 - 1 cannot be represented in type
    'long int'

As a minimal possible fix perform operation on unsigned integers where
overflow is well-defined, which was probably the original intent.

Signed-off-by: Marat Khalili <marat.khalili@huawei.com>
Acked-by: Konstantin Ananyev <konstantin.ananyev@huawei.com>
---
 app/test/test_bpf.c    | 56 ++++++++++++++++++++++++++++++++++++++++++
 lib/bpf/bpf_validate.c |  8 +++---
 2 files changed, 60 insertions(+), 4 deletions(-)

diff --git a/app/test/test_bpf.c b/app/test/test_bpf.c
index 6ecc49efff..c25f5a6f4d 100644
--- a/app/test/test_bpf.c
+++ b/app/test/test_bpf.c
@@ -152,6 +152,62 @@ test_minimal_working(void)
 
 REGISTER_FAST_TEST(bpf_minimal_working_autotest, true, true, test_minimal_working);
 
+/*
+ * Try and load valid BPF program adding one to the argument.
+ */
+static int
+test_add_one(void)
+{
+	static const struct ebpf_insn ins[] = {
+		{
+			/* Set return value to one. */
+			.code = (EBPF_ALU64 | EBPF_MOV | BPF_K),
+			.dst_reg = EBPF_REG_0,
+			.imm = 1,
+		},
+		{
+			/* Add program argument to the return value. */
+			.code = (EBPF_ALU64 | BPF_ADD | BPF_X),
+			.src_reg = EBPF_REG_1,
+			.dst_reg = EBPF_REG_0,
+		},
+		{
+			.code = (BPF_JMP | EBPF_EXIT),
+		},
+	};
+	return bpf_load_test(RTE_DIM(ins), ins, 0);
+}
+
+REGISTER_FAST_TEST(bpf_add_one_autotest, true, true, test_add_one);
+
+/*
+ * Try and load valid BPF program subtracting one from the argument.
+ */
+static int
+test_subtract_one(void)
+{
+	static const struct ebpf_insn ins[] = {
+		{
+			/* Subtract one from the program argument. */
+			.code = (EBPF_ALU64 | BPF_SUB | BPF_K),
+			.dst_reg = EBPF_REG_1,
+			.imm = 1,
+		},
+		{
+			/* Set return value to the result. */
+			.code = (EBPF_ALU64 | EBPF_MOV | BPF_X),
+			.src_reg = EBPF_REG_1,
+			.dst_reg = EBPF_REG_0,
+		},
+		{
+			.code = (BPF_JMP | EBPF_EXIT),
+		},
+	};
+	return bpf_load_test(RTE_DIM(ins), ins, 0);
+}
+
+REGISTER_FAST_TEST(bpf_subtract_one_autotest, true, true, test_subtract_one);
+
 /*
  * Basic functional tests for librte_bpf.
  * The main procedure - load eBPF program, execute it and
diff --git a/lib/bpf/bpf_validate.c b/lib/bpf/bpf_validate.c
index 23444b3eaa..47ad6fef0f 100644
--- a/lib/bpf/bpf_validate.c
+++ b/lib/bpf/bpf_validate.c
@@ -243,8 +243,8 @@ eval_add(struct bpf_reg_val *rd, const struct bpf_reg_val *rs, uint64_t msk)
 
 	rv.u.min = (rd->u.min + rs->u.min) & msk;
 	rv.u.max = (rd->u.max + rs->u.max) & msk;
-	rv.s.min = (rd->s.min + rs->s.min) & msk;
-	rv.s.max = (rd->s.max + rs->s.max) & msk;
+	rv.s.min = ((uint64_t)rd->s.min + (uint64_t)rs->s.min) & msk;
+	rv.s.max = ((uint64_t)rd->s.max + (uint64_t)rs->s.max) & msk;
 
 	/*
 	 * if at least one of the operands is not constant,
@@ -272,8 +272,8 @@ eval_sub(struct bpf_reg_val *rd, const struct bpf_reg_val *rs, uint64_t msk)
 
 	rv.u.min = (rd->u.min - rs->u.max) & msk;
 	rv.u.max = (rd->u.max - rs->u.min) & msk;
-	rv.s.min = (rd->s.min - rs->s.max) & msk;
-	rv.s.max = (rd->s.max - rs->s.min) & msk;
+	rv.s.min = ((uint64_t)rd->s.min - (uint64_t)rs->s.max) & msk;
+	rv.s.max = ((uint64_t)rd->s.max - (uint64_t)rs->s.min) & msk;
 
 	/*
 	 * if at least one of the operands is not constant,
-- 
2.43.0

[PATCH v2 5/5] bpf: fix BPF validation w/ conditional jump first

[Marat Khalili]

When the BPF program was starting with a conditional jump only one
(true) execution branch of the program was evaluated. Any instructions
jumped over were not evaluated and could contain invalid operations.
The root cause was using zero instruction index as a signal for ending
evaluation when backtracking.

Switch from using previous instruction index for tracking execution
history to a previous instruction pointer. First instruction will not
have it set, and therefore backtracking _from_ it will end evaluation,
not backtracking _to_ it like before.

Add two tests demostrating the problem:
* test_jump_over_invalid_first: loads BPF program with
  conditional jump over the invalid operation, should not succeeed;
* test_jump_over_invalid_non_first: same program with one extra
  instruction at the start to demostrate that it is indeed invalid
  (and also guard against another kind of regression);

Fixes: 6e12ec4c4d ("bpf: add more checks")

Signed-off-by: Marat Khalili <marat.khalili@huawei.com>
---
 app/test/test_bpf.c    | 80 ++++++++++++++++++++++++++++++++++++++++++
 lib/bpf/bpf_validate.c | 20 ++++-------
 2 files changed, 86 insertions(+), 14 deletions(-)

diff --git a/app/test/test_bpf.c b/app/test/test_bpf.c
index c25f5a6f4d..3f04cb28be 100644
--- a/app/test/test_bpf.c
+++ b/app/test/test_bpf.c
@@ -208,6 +208,86 @@ test_subtract_one(void)
 
 REGISTER_FAST_TEST(bpf_subtract_one_autotest, true, true, test_subtract_one);
 
+/*
+ * Conditionally jump over invalid operation as first instruction.
+ */
+static int
+test_jump_over_invalid_first(void)
+{
+	static const struct ebpf_insn ins[] = {
+		{
+			/* Jump over the next instruction for some r1. */
+			.code = (BPF_JMP | BPF_JEQ | BPF_K),
+			.dst_reg = EBPF_REG_1,
+			.imm = 42,
+			.off = 1,
+		},
+		{
+			/* Write 0xDEADBEEF to [r1 + INT16_MIN]. */
+			.code = (BPF_ST | BPF_MEM | EBPF_DW),
+			.dst_reg = EBPF_REG_1,
+			.off = INT16_MIN,
+			.imm = 0xDEADBEEF,
+		},
+		{
+			/* Set return value to the program argument. */
+			.code = (EBPF_ALU64 | EBPF_MOV | BPF_X),
+			.src_reg = EBPF_REG_1,
+			.dst_reg = EBPF_REG_0,
+		},
+		{
+			.code = (BPF_JMP | EBPF_EXIT),
+		},
+	};
+	return bpf_load_test(RTE_DIM(ins), ins, EINVAL);
+}
+
+REGISTER_FAST_TEST(bpf_jump_over_invalid_first_autotest, true, true,
+	test_jump_over_invalid_first);
+
+/*
+ * Conditionally jump over invalid operation as non-first instruction.
+ */
+static int
+test_jump_over_invalid_non_first(void)
+{
+	static const struct ebpf_insn ins[] = {
+		{
+			/* Set return value to the program argument. */
+			.code = (EBPF_ALU64 | EBPF_MOV | BPF_X),
+			.src_reg = EBPF_REG_1,
+			.dst_reg = EBPF_REG_0,
+		},
+		{
+			/* Jump over the next instruction for some r1. */
+			.code = (BPF_JMP | BPF_JEQ | BPF_K),
+			.dst_reg = EBPF_REG_1,
+			.imm = 42,
+			.off = 1,
+		},
+		{
+			/* Write 0xDEADBEEF to [r1 + INT16_MIN]. */
+			.code = (BPF_ST | BPF_MEM | EBPF_DW),
+			.dst_reg = EBPF_REG_1,
+			.off = INT16_MIN,
+			.imm = 0xDEADBEEF,
+		},
+		{
+			/* Set return value to the program argument. */
+			.code = (EBPF_ALU64 | EBPF_MOV | BPF_X),
+			.src_reg = EBPF_REG_1,
+			.dst_reg = EBPF_REG_0,
+		},
+		{
+			.code = (BPF_JMP | EBPF_EXIT),
+		},
+	};
+	return bpf_load_test(RTE_DIM(ins), ins, EINVAL);
+}
+
+REGISTER_FAST_TEST(bpf_jump_over_invalid_non_first_autotest, true, true,
+	test_jump_over_invalid_non_first);
+
 /*
  * Basic functional tests for librte_bpf.
  * The main procedure - load eBPF program, execute it and
diff --git a/lib/bpf/bpf_validate.c b/lib/bpf/bpf_validate.c
index 47ad6fef0f..64a8f227a3 100644
--- a/lib/bpf/bpf_validate.c
+++ b/lib/bpf/bpf_validate.c
@@ -64,7 +64,7 @@ struct inst_node {
 	uint8_t cur_edge:4;
 	uint8_t edge_type[MAX_EDGES];
 	uint32_t edge_dest[MAX_EDGES];
-	uint32_t prev_node;
+	struct inst_node *prev_node;
 	struct {
 		struct bpf_eval_state *cur;   /* save/restore for jcc targets */
 		struct bpf_eval_state *start;
@@ -1875,12 +1875,6 @@ set_edge_type(struct bpf_verifier *bvf, struct inst_node *node,
 	bvf->edge_type[type]++;
 }
 
-static struct inst_node *
-get_prev_node(struct bpf_verifier *bvf, struct inst_node *node)
-{
-	return  bvf->in + node->prev_node;
-}
-
 /*
  * Depth-First Search (DFS) through previously constructed
  * Control Flow Graph (CFG).
@@ -1916,7 +1910,7 @@ dfs(struct bpf_verifier *bvf)
 
 			if (next != NULL) {
 				/* proceed with next child */
-				next->prev_node = get_node_idx(bvf, node);
+				next->prev_node = node;
 				node = next;
 			} else {
 				/*
@@ -1925,7 +1919,7 @@ dfs(struct bpf_verifier *bvf)
 				 */
 				set_node_colour(bvf, node, BLACK);
 				node->cur_edge = 0;
-				node = get_prev_node(bvf, node);
+				node = node->prev_node;
 			}
 		} else
 			node = NULL;
@@ -2490,7 +2484,7 @@ evaluate(struct bpf_verifier *bvf)
 				next = NULL;
 				stats.nb_prune++;
 			} else {
-				next->prev_node = get_node_idx(bvf, node);
+				next->prev_node = node;
 				node = next;
 			}
 		} else {
@@ -2501,11 +2495,9 @@ evaluate(struct bpf_verifier *bvf)
 			 */
 			node->cur_edge = 0;
 			save_safe_eval_state(bvf, node);
-			node = get_prev_node(bvf, node);
+			node = node->prev_node;
 
-			/* finished */
-			if (node == bvf->in)
-				node = NULL;
+			/* first node will not have prev, signalling finish */
 		}
 	}
 
-- 
2.43.0

RE: [PATCH v2 3/5] bpf: disallow empty program

[Marat Khalili]

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

> I see no need for docbook style comments for static functions.
> Especially in test only code

Thank you, will remember it and remove and next iteration.

[-- Attachment #2: Type: text/html, Size: 1867 bytes --]

RE: [PATCH v2 1/5] eal: variable first arguments of RTE_SHIFT_VALxx

[Morten Brørup]

> From: Marat Khalili [mailto:marat.khalili@huawei.com]
> Sent: Tuesday, 16 December 2025 19.21
> 
> Macros RTE_SHIFT_VAL32 and RTE_SHIFT_VAL64 were implemented by applying
> UINT32_C or UINT64_C correspondingly to its first argument. As a
> consequence first argument had to be a constant. Replace UINT32_C and
> UINT64_C with casts to uint32_t and uint64_t to allow these arguments
> be
> variable. For constants the result should be the same.
> 
> (Yes, technically UINT64_C promotes to uint_least64_t, not uint64_t,
> but
> I think most users of RTE_SHIFT_VAL64 expect the result to be
> uint64_t.)

Interesting detail.
And yes, I agree about expectations.

> 
> Signed-off-by: Marat Khalili <marat.khalili@huawei.com>
> ---
>  lib/eal/include/rte_bitops.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/eal/include/rte_bitops.h
> b/lib/eal/include/rte_bitops.h
> index 2d1b9d281c..aa6ac73abb 100644
> --- a/lib/eal/include/rte_bitops.h
> +++ b/lib/eal/include/rte_bitops.h
> @@ -51,7 +51,7 @@ extern "C" {
>   * @param nr
>   *   The shift number in range of 0 to (32 - width of val).
>   */
> -#define RTE_SHIFT_VAL32(val, nr) (UINT32_C(val) << (nr))
> +#define RTE_SHIFT_VAL32(val, nr) ((uint32_t)(val) << (nr))
> 
>  /**
>   * Get the uint64_t shifted value.
> @@ -61,7 +61,7 @@ extern "C" {
>   * @param nr
>   *   The shift number in range of 0 to (64 - width of val).
>   */
> -#define RTE_SHIFT_VAL64(val, nr) (UINT64_C(val) << (nr))
> +#define RTE_SHIFT_VAL64(val, nr) ((uint64_t)(val) << (nr))
> 
>  /**
>   * Generate a contiguous 32-bit mask
> --
> 2.43.0

Reviewed-by: Morten Brørup <mb@smartsharesystems.com>

RE: [PATCH v2 2/5] bpf: fix signed shift overflows in ARM JIT

[Morten Brørup]

> From: Marat Khalili [mailto:marat.khalili@huawei.com]
> Sent: Tuesday, 16 December 2025 19.21
> 
> Left shifts of integer literals and bool values overwriting the sign
> bit
> were used multiple times in bpf_jit_arm64.c. E.g.:
> 
>     insn = (!!is64) << 31;
> 
> where is64 has type bool (double bang is a no-op here).

Yes, multiple online sources [1] confirms that the bool type can only hold two values, 0 and 1, and converting any non-zero value to bool results in 1.

So, using !!x where x is a bool is literally a no-op, since x - being a bool - could already only hold the values 0 and 1.

[1]: E.g.
https://www.gnu.org/software/c-intro-and-ref/manual/html_node/Boolean-Type.html
https://embedded.fm/blog/2016/5/3/ew-bools

> The operand of
> left shift was promoted to type int, which when 32-bit wide cannot
> represent the result. Similarly literal integers have int type by
> default.  Sanitizer produced the following diagnostic during runtime
> (for various lines):
> 
>     lib/bpf/bpf_jit_arm64.c:241:18: runtime error: left shift of 1 by
> 31
>     places cannot be represented in type 'int'
> 
> To fix the issue use RTE_BIT32 and similar macros instead.
> 
> Signed-off-by: Marat Khalili <marat.khalili@huawei.com>
> ---
>  lib/bpf/bpf_jit_arm64.c | 162 ++++++++++++++++++++--------------------
>  1 file changed, 81 insertions(+), 81 deletions(-)
> 
> diff --git a/lib/bpf/bpf_jit_arm64.c b/lib/bpf/bpf_jit_arm64.c
> index 96b8cd2e03..76df1e4ba1 100644
> --- a/lib/bpf/bpf_jit_arm64.c
> +++ b/lib/bpf/bpf_jit_arm64.c
> @@ -28,7 +28,7 @@
>  #define A64_ZR			31
> 
>  #define check_imm(n, val) (((val) >= 0) ? !!((val) >> (n)) : !!((~val)
> >> (n)))
> -#define mask_imm(n, val) ((val) & ((1 << (n)) - 1))
> +#define mask_imm(n, val) ((val) & (RTE_BIT32(n) - 1))
> 
>  struct ebpf_a64_map {
>  	uint32_t off; /* eBPF to arm64 insn offset mapping for jump */
> @@ -238,12 +238,12 @@ emit_add_sub_imm(struct a64_jit_ctx *ctx, bool
> is64, bool sub, uint8_t rd,
>  	uint32_t insn, imm;
> 
>  	imm = mask_imm(12, imm12);
> -	insn = (!!is64) << 31;
> -	insn |= (!!sub) << 30;
> +	insn = RTE_SHIFT_VAL32(is64, 31);
> +	insn |= RTE_SHIFT_VAL32(sub, 30);

As mentioned above, !!is64 and !!sub are literally no-ops, so it is safe to remove them.

>  	insn |= 0x11000000;
>  	insn |= rd;
> -	insn |= rn << 5;
> -	insn |= imm << 10;
> +	insn |= RTE_SHIFT_VAL32(rn, 5);
> +	insn |= RTE_SHIFT_VAL32(imm, 10);
> 
>  	emit_insn(ctx, insn,
>  		  check_reg(rd) || check_reg(rn) || check_imm(12, imm12));
> @@ -279,16 +279,16 @@ emit_ls_pair_64(struct a64_jit_ctx *ctx, uint8_t
> rt, uint8_t rt2, uint8_t rn,
>  {
>  	uint32_t insn;
> 
> -	insn = (!!load) << 22;
> -	insn |= (!!pre_index) << 24;
> +	insn = RTE_SHIFT_VAL32(load, 22);
> +	insn |= RTE_SHIFT_VAL32(pre_index, 24);
>  	insn |= 0xa8800000;

This constant has the high bit set, consider:
insn |= UINT32_C(0xa8800000);

And for consistency, consider also doing it for constants where the high bit isn't set.

Throughout the file.

>  	insn |= rt;
> -	insn |= rn << 5;
> -	insn |= rt2 << 10;
> +	insn |= RTE_SHIFT_VAL32(rn, 5);
> +	insn |= RTE_SHIFT_VAL32(rt2, 10);
>  	if (push)
> -		insn |= 0x7e << 15; /* 0x7e means -2 with imm7 */
> +		insn |= RTE_SHIFT_VAL32(0x7e, 15); /* 0x7e means -2 with
> imm7 */
>  	else
> -		insn |= 0x2 << 15;
> +		insn |= RTE_SHIFT_VAL32(0x2, 15);
> 
>  	emit_insn(ctx, insn, check_reg(rn) || check_reg(rt) ||
> check_reg(rt2));
> 
> @@ -317,11 +317,11 @@ mov_imm(struct a64_jit_ctx *ctx, bool is64,
> uint8_t rd, uint8_t type,
>  {
>  	uint32_t insn;
> 
> -	insn = (!!is64) << 31;
> -	insn |= type << 29;
> -	insn |= 0x25 << 23;
> -	insn |= (shift/16) << 21;
> -	insn |= imm16 << 5;
> +	insn = RTE_SHIFT_VAL32(is64, 31);
> +	insn |= RTE_SHIFT_VAL32(type, 29);
> +	insn |= RTE_SHIFT_VAL32(0x25, 23);
> +	insn |= RTE_SHIFT_VAL32(shift/16, 21);
> +	insn |= RTE_SHIFT_VAL32(imm16, 5);
>  	insn |= rd;
> 
>  	emit_insn(ctx, insn, check_reg(rd) || check_mov_hw(is64, shift));
> @@ -334,7 +334,7 @@ emit_mov_imm32(struct a64_jit_ctx *ctx, bool is64,
> uint8_t rd, uint32_t val)
>  	uint16_t lower = val & 0xffff;
> 
>  	/* Positive number */
> -	if ((val & 1UL << 31) == 0) {
> +	if ((val & RTE_BIT32(31)) == 0) {
>  		mov_imm(ctx, is64, rd, A64_MOVZ, lower, 0);
>  		if (upper)
>  			mov_imm(ctx, is64, rd, A64_MOVK, upper, 16);
> @@ -393,21 +393,21 @@ emit_ls(struct a64_jit_ctx *ctx, uint8_t sz,
> uint8_t rt, uint8_t rn, uint8_t rm,
>  {
>  	uint32_t insn;
> 
> -	insn = 0x1c1 << 21;
> +	insn = RTE_SHIFT_VAL32(0x1c1, 21);
>  	if (load)
> -		insn |= 1 << 22;
> +		insn |= RTE_BIT32(22);
>  	if (sz == BPF_B)
> -		insn |= 0 << 30;
> +		insn |= RTE_SHIFT_VAL32(0, 30);
>  	else if (sz == BPF_H)
> -		insn |= 1 << 30;
> +		insn |= RTE_SHIFT_VAL32(1, 30);
>  	else if (sz == BPF_W)
> -		insn |= 2 << 30;
> +		insn |= RTE_SHIFT_VAL32(2, 30);
>  	else if (sz == EBPF_DW)
> -		insn |= 3 << 30;
> +		insn |= RTE_SHIFT_VAL32(3, 30);
> 
> -	insn |= rm << 16;
> -	insn |= 0x1a << 10; /* LSL and S = 0 */
> -	insn |= rn << 5;
> +	insn |= RTE_SHIFT_VAL32(rm, 16);
> +	insn |= RTE_SHIFT_VAL32(0x1a, 10); /* LSL and S = 0 */
> +	insn |= RTE_SHIFT_VAL32(rn, 5);
>  	insn |= rt;
> 
>  	emit_insn(ctx, insn, check_reg(rt) || check_reg(rn) ||
> check_reg(rm) ||
> @@ -436,10 +436,10 @@ emit_add_sub(struct a64_jit_ctx *ctx, bool is64,
> uint8_t rd, uint8_t rn,
>  {
>  	uint32_t insn;
> 
> -	insn = (!!is64) << 31;
> -	insn |= op << 21; /* shift == 0 */
> -	insn |= rm << 16;
> -	insn |= rn << 5;
> +	insn = RTE_SHIFT_VAL32(is64, 31);
> +	insn |= RTE_SHIFT_VAL32(op, 21); /* shift == 0 */
> +	insn |= RTE_SHIFT_VAL32(rm, 16);
> +	insn |= RTE_SHIFT_VAL32(rn, 5);
>  	insn |= rd;
> 
>  	emit_insn(ctx, insn, check_reg(rd) || check_reg(rm));
> @@ -468,11 +468,11 @@ emit_mul(struct a64_jit_ctx *ctx, bool is64,
> uint8_t rd, uint8_t rm)
>  {
>  	uint32_t insn;
> 
> -	insn = (!!is64) << 31;
> -	insn |= 0xd8 << 21;
> -	insn |= rm << 16;
> -	insn |= A64_ZR << 10;
> -	insn |= rd << 5;
> +	insn = RTE_SHIFT_VAL32(is64, 31);
> +	insn |= RTE_SHIFT_VAL32(0xd8, 21);
> +	insn |= RTE_SHIFT_VAL32(rm, 16);
> +	insn |= RTE_SHIFT_VAL32(A64_ZR, 10);
> +	insn |= RTE_SHIFT_VAL32(rd, 5);
>  	insn |= rd;
> 
>  	emit_insn(ctx, insn, check_reg(rd) || check_reg(rm));
> @@ -489,11 +489,11 @@ emit_data_process_two_src(struct a64_jit_ctx
> *ctx, bool is64, uint8_t rd,
>  {
>  	uint32_t insn;
> 
> -	insn = (!!is64) << 31;
> -	insn |= 0xd6 << 21;
> -	insn |= rm << 16;
> -	insn |= op << 10;
> -	insn |= rn << 5;
> +	insn = RTE_SHIFT_VAL32(is64, 31);
> +	insn |= RTE_SHIFT_VAL32(0xd6, 21);
> +	insn |= RTE_SHIFT_VAL32(rm, 16);
> +	insn |= RTE_SHIFT_VAL32(op, 10);
> +	insn |= RTE_SHIFT_VAL32(rn, 5);
>  	insn |= rd;
> 
>  	emit_insn(ctx, insn, check_reg(rd) || check_reg(rm));
> @@ -532,14 +532,14 @@ emit_bitfield(struct a64_jit_ctx *ctx, bool is64,
> uint8_t rd, uint8_t rn,
>  {
>  	uint32_t insn;
> 
> -	insn = (!!is64) << 31;
> +	insn = RTE_SHIFT_VAL32(is64, 31);
>  	if (insn)
> -		insn |= 1 << 22; /* Set N bit when is64 is set */
> -	insn |= op << 29;
> -	insn |= 0x26 << 23;
> -	insn |= immr << 16;
> -	insn |= imms << 10;
> -	insn |= rn << 5;
> +		insn |= RTE_BIT32(22); /* Set N bit when is64 is set */
> +	insn |= RTE_SHIFT_VAL32(op, 29);
> +	insn |= RTE_SHIFT_VAL32(0x26, 23);
> +	insn |= RTE_SHIFT_VAL32(immr, 16);
> +	insn |= RTE_SHIFT_VAL32(imms, 10);
> +	insn |= RTE_SHIFT_VAL32(rn, 5);
>  	insn |= rd;
> 
>  	emit_insn(ctx, insn, check_reg(rd) || check_reg(rn) ||
> @@ -578,11 +578,11 @@ emit_logical(struct a64_jit_ctx *ctx, bool is64,
> uint8_t rd,
>  {
>  	uint32_t insn;
> 
> -	insn = (!!is64) << 31;
> -	insn |= op << 29;
> -	insn |= 0x50 << 21;
> -	insn |= rm << 16;
> -	insn |= rd << 5;
> +	insn = RTE_SHIFT_VAL32(is64, 31);
> +	insn |= RTE_SHIFT_VAL32(op, 29);
> +	insn |= RTE_SHIFT_VAL32(0x50, 21);
> +	insn |= RTE_SHIFT_VAL32(rm, 16);
> +	insn |= RTE_SHIFT_VAL32(rd, 5);
>  	insn |= rd;
> 
>  	emit_insn(ctx, insn, check_reg(rd) || check_reg(rm));
> @@ -612,12 +612,12 @@ emit_msub(struct a64_jit_ctx *ctx, bool is64,
> uint8_t rd, uint8_t rn,
>  {
>  	uint32_t insn;
> 
> -	insn = (!!is64) << 31;
> -	insn |= 0xd8 << 21;
> -	insn |= rm << 16;
> -	insn |= 0x1 << 15;
> -	insn |= ra << 10;
> -	insn |= rn << 5;
> +	insn = RTE_SHIFT_VAL32(is64, 31);
> +	insn |= RTE_SHIFT_VAL32(0xd8, 21);
> +	insn |= RTE_SHIFT_VAL32(rm, 16);
> +	insn |= RTE_SHIFT_VAL32(0x1, 15);
> +	insn |= RTE_SHIFT_VAL32(ra, 10);
> +	insn |= RTE_SHIFT_VAL32(rn, 5);
>  	insn |= rd;
> 
>  	emit_insn(ctx, insn, check_reg(rd) || check_reg(rn) ||
> check_reg(rm) ||
> @@ -638,7 +638,7 @@ emit_blr(struct a64_jit_ctx *ctx, uint8_t rn)
>  	uint32_t insn;
> 
>  	insn = 0xd63f0000;
> -	insn |= rn << 5;
> +	insn |= RTE_SHIFT_VAL32(rn, 5);
> 
>  	emit_insn(ctx, insn, check_reg(rn));
>  }
> @@ -669,22 +669,22 @@ emit_rev(struct a64_jit_ctx *ctx, uint8_t rd,
> int32_t imm)
>  	uint32_t insn;
> 
>  	insn = 0xdac00000;
> -	insn |= rd << 5;
> +	insn |= RTE_SHIFT_VAL32(rd, 5);
>  	insn |= rd;
> 
>  	switch (imm) {
>  	case 16:
> -		insn |= 1 << 10;
> +		insn |= RTE_SHIFT_VAL32(1, 10);
>  		emit_insn(ctx, insn, check_reg(rd));
>  		emit_zero_extend(ctx, rd, 16);
>  		break;
>  	case 32:
> -		insn |= 2 << 10;
> +		insn |= RTE_SHIFT_VAL32(2, 10);
>  		emit_insn(ctx, insn, check_reg(rd));
>  		/* Upper 32 bits already cleared */
>  		break;
>  	case 64:
> -		insn |= 3 << 10;
> +		insn |= RTE_SHIFT_VAL32(3, 10);
>  		emit_insn(ctx, insn, check_reg(rd));
>  		break;
>  	default:
> @@ -933,9 +933,9 @@ emit_cbnz(struct a64_jit_ctx *ctx, bool is64,
> uint8_t rt, int32_t imm19)
>  	uint32_t insn, imm;
> 
>  	imm = mask_imm(19, imm19);
> -	insn = (!!is64) << 31;
> -	insn |= 0x35 << 24;
> -	insn |= imm << 5;
> +	insn = RTE_SHIFT_VAL32(is64, 31);
> +	insn |= RTE_SHIFT_VAL32(0x35, 24);
> +	insn |= RTE_SHIFT_VAL32(imm, 5);
>  	insn |= rt;
> 
>  	emit_insn(ctx, insn, check_reg(rt) || check_imm(19, imm19));
> @@ -947,7 +947,7 @@ emit_b(struct a64_jit_ctx *ctx, int32_t imm26)
>  	uint32_t insn, imm;
> 
>  	imm = mask_imm(26, imm26);
> -	insn = 0x5 << 26;
> +	insn = RTE_SHIFT_VAL32(0x5, 26);
>  	insn |= imm;
> 
>  	emit_insn(ctx, insn, check_imm(26, imm26));
> @@ -971,9 +971,9 @@ emit_stadd(struct a64_jit_ctx *ctx, bool is64,
> uint8_t rs, uint8_t rn)
>  	uint32_t insn;
> 
>  	insn = 0xb820001f;
> -	insn |= (!!is64) << 30;
> -	insn |= rs << 16;
> -	insn |= rn << 5;
> +	insn |= RTE_SHIFT_VAL32(is64, 30);
> +	insn |= RTE_SHIFT_VAL32(rs, 16);
> +	insn |= RTE_SHIFT_VAL32(rn, 5);
> 
>  	emit_insn(ctx, insn, check_reg(rs) || check_reg(rn));
>  }
> @@ -984,8 +984,8 @@ emit_ldxr(struct a64_jit_ctx *ctx, bool is64,
> uint8_t rt, uint8_t rn)
>  	uint32_t insn;
> 
>  	insn = 0x885f7c00;
> -	insn |= (!!is64) << 30;
> -	insn |= rn << 5;
> +	insn |= RTE_SHIFT_VAL32(is64, 30);
> +	insn |= RTE_SHIFT_VAL32(rn, 5);
>  	insn |= rt;
> 
>  	emit_insn(ctx, insn, check_reg(rt) || check_reg(rn));
> @@ -998,9 +998,9 @@ emit_stxr(struct a64_jit_ctx *ctx, bool is64,
> uint8_t rs, uint8_t rt,
>  	uint32_t insn;
> 
>  	insn = 0x88007c00;
> -	insn |= (!!is64) << 30;
> -	insn |= rs << 16;
> -	insn |= rn << 5;
> +	insn |= RTE_SHIFT_VAL32(is64, 30);
> +	insn |= RTE_SHIFT_VAL32(rs, 16);
> +	insn |= RTE_SHIFT_VAL32(rn, 5);
>  	insn |= rt;
> 
>  	emit_insn(ctx, insn, check_reg(rs) || check_reg(rt) ||
> check_reg(rn));
> @@ -1051,9 +1051,9 @@ emit_cmp_tst(struct a64_jit_ctx *ctx, bool is64,
> uint8_t rn, uint8_t rm,
>  	uint32_t insn;
> 
>  	insn = opc;
> -	insn |= (!!is64) << 31;
> -	insn |= rm << 16;
> -	insn |= rn << 5;
> +	insn |= RTE_SHIFT_VAL32(is64, 31);
> +	insn |= RTE_SHIFT_VAL32(rm, 16);
> +	insn |= RTE_SHIFT_VAL32(rn, 5);
> 
>  	emit_insn(ctx, insn, check_reg(rn) || check_reg(rm));
>  }
> @@ -1076,8 +1076,8 @@ emit_b_cond(struct a64_jit_ctx *ctx, uint8_t
> cond, int32_t imm19)
>  	uint32_t insn, imm;
> 
>  	imm = mask_imm(19, imm19);
> -	insn = 0x15 << 26;
> -	insn |= imm << 5;
> +	insn = RTE_SHIFT_VAL32(0x15, 26);
> +	insn |= RTE_SHIFT_VAL32(imm, 5);
>  	insn |= cond;
> 
>  	emit_insn(ctx, insn, check_cond(cond) || check_imm(19, imm19));
> @@ -1301,7 +1301,7 @@ emit(struct a64_jit_ctx *ctx, struct rte_bpf
> *bpf)
>  			break;
>  		/* dst = imm64 */
>  		case (BPF_LD | BPF_IMM | EBPF_DW):
> -			u64 = ((uint64_t)ins[1].imm << 32) | (uint32_t)imm;
> +			u64 = RTE_SHIFT_VAL64(ins[1].imm, 32) |
> (uint32_t)imm;
>  			emit_mov_imm(ctx, 1, dst, u64);
>  			i++;
>  			break;
> --
> 2.43.0

With or without suggested changes,
Acked-by: Morten Brørup <mb@smartsharesystems.com>

Re: [PATCH v2 3/5] bpf: disallow empty program

[Stephen Hemminger]

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

I see no need for docbook style comments for static functions.
Especially in test only code

On Tue, Dec 16, 2025, 10:21 Marat Khalili <marat.khalili@huawei.com> wrote:

> Add tests for some simple cases:
> * Program with no instructions;
> * Program with only EXIT instruction but no return value set;
> * Program with return value set but no EXIT instruction;
> * Minimal valid program with return value set and an EXIT instruction.
>
> Fix found bugs:
> * a program with no instructions was accepted;
> * a program with no EXIT instruction read outside the buffer.
>
> Signed-off-by: Marat Khalili <marat.khalili@huawei.com>
> Acked-by: Konstantin Ananyev <konstantin.ananyev@huawei.com>
> ---
>  app/test/test_bpf.c    | 118 +++++++++++++++++++++++++++++++++++++++++
>  lib/bpf/bpf_load.c     |   2 +-
>  lib/bpf/bpf_validate.c |  20 +++++--
>  3 files changed, 135 insertions(+), 5 deletions(-)
>
> diff --git a/app/test/test_bpf.c b/app/test/test_bpf.c
> index b7c94ba1c7..6ecc49efff 100644
> --- a/app/test/test_bpf.c
> +++ b/app/test/test_bpf.c
> @@ -34,6 +34,124 @@ test_bpf(void)
>  #include <rte_ip.h>
>
>
> +/* Tests of most simple BPF programs (no instructions, one instruction
> etc.) */
> +
> +/*
> + * Try to load a simple bpf program from the instructions array.
> + *
> + * When `expected_errno` is zero, expect it to load successfully.
> + * When `expected_errno` is non-zero, expect it to fail with this
> `rte_errno`.
> + *
> + * @param nb_ins
> + *   Number of instructions in the `ins` array.
> + * @param ins
> + *   BPF instructions array.
> + * @param expected_errno
> + *   Expected result.
> + * @return
> + *   TEST_SUCCESS on success, error code on failure.
> + */
> +static int
> +bpf_load_test(uint32_t nb_ins, const struct ebpf_insn *ins, int
> expected_errno)
> +{
> +       const struct rte_bpf_prm prm = {
> +               .ins = ins,
> +               .nb_ins = nb_ins,
> +               .prog_arg = {
> +                       .type = RTE_BPF_ARG_RAW,
> +                       .size = sizeof(uint64_t),
> +               },
> +       };
> +
> +       struct rte_bpf *const bpf = rte_bpf_load(&prm);
> +       const int actual_errno = rte_errno;
> +       rte_bpf_destroy(bpf);
> +
> +       if (expected_errno != 0) {
> +               RTE_TEST_ASSERT_EQUAL(bpf, NULL,
> +                       "expect rte_bpf_load() == NULL");
> +               RTE_TEST_ASSERT_EQUAL(actual_errno, expected_errno,
> +                       "expect rte_errno == %d, found %d",
> +                       expected_errno, actual_errno);
> +       } else
> +               RTE_TEST_ASSERT_NOT_EQUAL(bpf, NULL,
> +                       "expect rte_bpf_load() != NULL");
> +
> +       return TEST_SUCCESS;
> +}
> +
> +/*
> + * Try and load completely empty BPF program.
> + * Should fail because there is no EXIT (and also return value is
> undefined).
> + */
> +static int
> +test_no_instructions(void)
> +{
> +       static const struct ebpf_insn ins[] = {};
> +       return bpf_load_test(RTE_DIM(ins), ins, EINVAL);
> +}
> +
> +REGISTER_FAST_TEST(bpf_no_instructions_autotest, true, true,
> test_no_instructions);
> +
> +/*
> + * Try and load a BPF program comprising single EXIT instruction.
> + * Should fail because the return value is undefined.
> + */
> +static int
> +test_exit_only(void)
> +{
> +       static const struct ebpf_insn ins[] = {
> +               {
> +                       .code = (BPF_JMP | EBPF_EXIT),
> +               },
> +       };
> +       return bpf_load_test(RTE_DIM(ins), ins, EINVAL);
> +}
> +
> +REGISTER_FAST_TEST(bpf_exit_only_autotest, true, true, test_exit_only);
> +
> +/*
> + * Try and load a BPF program with no EXIT instruction.
> + * Should fail because of this.
> + */
> +static int
> +test_no_exit(void)
> +{
> +       static const struct ebpf_insn ins[] = {
> +               {
> +                       /* Set return value to the program argument. */
> +                       .code = (EBPF_ALU64 | EBPF_MOV | BPF_X),
> +                       .src_reg = EBPF_REG_1,
> +                       .dst_reg = EBPF_REG_0,
> +               },
> +       };
> +       return bpf_load_test(RTE_DIM(ins), ins, EINVAL);
> +}
> +
> +REGISTER_FAST_TEST(bpf_no_exit_autotest, true, true, test_no_exit);
> +
> +/*
> + * Try and load smallest possible valid BPF program.
> + */
> +static int
> +test_minimal_working(void)
> +{
> +       static const struct ebpf_insn ins[] = {
> +               {
> +                       /* Set return value to the program argument. */
> +                       .code = (EBPF_ALU64 | EBPF_MOV | BPF_X),
> +                       .src_reg = EBPF_REG_1,
> +                       .dst_reg = EBPF_REG_0,
> +               },
> +               {
> +                       .code = (BPF_JMP | EBPF_EXIT),
> +               },
> +       };
> +       return bpf_load_test(RTE_DIM(ins), ins, 0);
> +}
> +
> +REGISTER_FAST_TEST(bpf_minimal_working_autotest, true, true,
> test_minimal_working);
> +
>  /*
>   * Basic functional tests for librte_bpf.
>   * The main procedure - load eBPF program, execute it and
> diff --git a/lib/bpf/bpf_load.c b/lib/bpf/bpf_load.c
> index 556e613762..6983c026af 100644
> --- a/lib/bpf/bpf_load.c
> +++ b/lib/bpf/bpf_load.c
> @@ -88,7 +88,7 @@ rte_bpf_load(const struct rte_bpf_prm *prm)
>         int32_t rc;
>         uint32_t i;
>
> -       if (prm == NULL || prm->ins == NULL ||
> +       if (prm == NULL || prm->ins == NULL || prm->nb_ins == 0 ||
>                         (prm->nb_xsym != 0 && prm->xsym == NULL)) {
>                 rte_errno = EINVAL;
>                 return NULL;
> diff --git a/lib/bpf/bpf_validate.c b/lib/bpf/bpf_validate.c
> index 4f47d6dc7b..23444b3eaa 100644
> --- a/lib/bpf/bpf_validate.c
> +++ b/lib/bpf/bpf_validate.c
> @@ -1827,7 +1827,7 @@ add_edge(struct bpf_verifier *bvf, struct inst_node
> *node, uint32_t nidx)
>  {
>         uint32_t ne;
>
> -       if (nidx > bvf->prm->nb_ins) {
> +       if (nidx >= bvf->prm->nb_ins) {
>                 RTE_BPF_LOG_LINE(ERR,
>                         "%s: program boundary violation at pc: %u, next
> pc: %u",
>                         __func__, get_node_idx(bvf, node), nidx);
> @@ -1886,14 +1886,20 @@ get_prev_node(struct bpf_verifier *bvf, struct
> inst_node *node)
>   * Control Flow Graph (CFG).
>   * Information collected at this path would be used later
>   * to determine is there any loops, and/or unreachable instructions.
> + * PREREQUISITE: there is at least one node.
>   */
>  static void
>  dfs(struct bpf_verifier *bvf)
>  {
>         struct inst_node *next, *node;
>
> -       node = bvf->in;
> -       while (node != NULL) {
> +       RTE_ASSERT(bvf->nb_nodes != 0);
> +       /*
> +        * Since there is at least one node, node with index 0 always
> exists;
> +        * it is our program entry point.
> +        */
> +       node = &bvf->in[0];
> +       do {
>
>                 if (node->colour == WHITE)
>                         set_node_colour(bvf, node, GREY);
> @@ -1923,7 +1929,7 @@ dfs(struct bpf_verifier *bvf)
>                         }
>                 } else
>                         node = NULL;
> -       }
> +       } while (node != NULL);
>  }
>
>  /*
> @@ -2062,6 +2068,12 @@ validate(struct bpf_verifier *bvf)
>         if (rc != 0)
>                 return rc;
>
> +       if (bvf->nb_nodes == 0) {
> +               RTE_BPF_LOG_LINE(ERR, "%s(%p) the program is empty",
> +                       __func__, bvf);
> +               return -EINVAL;
> +       }
> +
>         dfs(bvf);
>
>         RTE_LOG(DEBUG, BPF, "%s(%p) stats:\n"
> --
> 2.43.0
>
>

[-- Attachment #2: Type: text/html, Size: 9549 bytes --]