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