[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.