[LTP] [PATCH v2 0/5] BPF refactor and add bpf_prog05

[LTP] [PATCH v2 0/5] BPF refactor and add bpf_prog05

[Richard Palethorpe]

Hello,

V2:

* Added a number of helper functions to bpf_common and used them on
  all tests.

* Added some instruction concatenation helpers, but only used these on
  bpf_prog05 as they don't seem to make the other tests better to
  read.

* Further shorten bpf_prog05 by using a generic expect function

Richard Palethorpe (5):
  lapi/bpf: Add /=, %= and BPF_MAXINSNS
  bpf: Add map_array helper functions
  bpf: Add helper to run socket programs
  bpf: Add bpf_insn_buf, map and instruction concatenation helpers
  bpf: Check truncation on 32bit div/mod by zero

 include/lapi/bpf.h                         |   6 +
 runtest/cve                                |   1 +
 runtest/syscalls                           |   1 +
 testcases/kernel/syscalls/bpf/.gitignore   |   1 +
 testcases/kernel/syscalls/bpf/bpf_common.c |  89 +++++++++++
 testcases/kernel/syscalls/bpf/bpf_common.h |  26 ++-
 testcases/kernel/syscalls/bpf/bpf_prog01.c |  32 +---
 testcases/kernel/syscalls/bpf/bpf_prog02.c |  54 ++-----
 testcases/kernel/syscalls/bpf/bpf_prog03.c |  32 +---
 testcases/kernel/syscalls/bpf/bpf_prog04.c |  17 +-
 testcases/kernel/syscalls/bpf/bpf_prog05.c | 175 +++++++++++++++++++++
 11 files changed, 320 insertions(+), 114 deletions(-)
 create mode 100644 testcases/kernel/syscalls/bpf/bpf_prog05.c

-- 
2.31.1

[LTP] [PATCH v2 1/5] lapi/bpf: Add /=, %= and BPF_MAXINSNS

[Richard Palethorpe]

Add div and mod instructions and the max program size.

Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com>
---
 include/lapi/bpf.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/include/lapi/bpf.h b/include/lapi/bpf.h
index f27a92146..0a25edb73 100644
--- a/include/lapi/bpf.h
+++ b/include/lapi/bpf.h
@@ -37,8 +37,10 @@
 #define BPF_OP(code)    ((code) & 0xf0)
 #define		BPF_ADD		0x00
 #define		BPF_SUB		0x10
+#define		BPF_DIV		0x30
 #define		BPF_LSH		0x60
 #define		BPF_RSH		0x70
+#define		BPF_MOD		0x90
 
 #define		BPF_JEQ		0x10
 
@@ -46,6 +48,10 @@
 #define		BPF_K		0x00
 #define		BPF_X		0x08
 
+#ifndef BPF_MAXINSNS
+#define BPF_MAXINSNS 4096
+#endif
+
 #define BPF_ALU64	0x07	/* alu mode in double word width */
 #define BPF_MOV		0xb0	/* mov reg to reg */
 #define BPF_CALL	0x80	/* function call */
-- 
2.31.1

[LTP] [PATCH v2 2/5] bpf: Add map_array helper functions

[Richard Palethorpe]

We usually just create an array map with standard key and value
size. Then fetch some values from the array. So we can create some
standard functions for this.

Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com>
---
 testcases/kernel/syscalls/bpf/bpf_common.c | 34 ++++++++++++++++
 testcases/kernel/syscalls/bpf/bpf_common.h | 12 +++++-
 testcases/kernel/syscalls/bpf/bpf_prog01.c | 25 +++---------
 testcases/kernel/syscalls/bpf/bpf_prog02.c | 47 +++++-----------------
 testcases/kernel/syscalls/bpf/bpf_prog03.c | 25 +++---------
 testcases/kernel/syscalls/bpf/bpf_prog04.c |  8 +---
 6 files changed, 68 insertions(+), 83 deletions(-)

diff --git a/testcases/kernel/syscalls/bpf/bpf_common.c b/testcases/kernel/syscalls/bpf/bpf_common.c
index fd299b73d..90772815b 100644
--- a/testcases/kernel/syscalls/bpf/bpf_common.c
+++ b/testcases/kernel/syscalls/bpf/bpf_common.c
@@ -48,6 +48,40 @@ int bpf_map_create(union bpf_attr *attr)
 	return ret;
 }
 
+int bpf_map_array_create(uint32_t max_entries)
+{
+	union bpf_attr map_attr = {
+		.map_type = BPF_MAP_TYPE_ARRAY,
+		.key_size = 4,
+		.value_size = 8,
+		.max_entries = max_entries,
+		.map_flags = 0
+	};
+
+	return bpf_map_create(&map_attr);
+}
+
+long bpf_map_array_get(const int map_fd,
+		       const uint32_t *const array_indx,
+		       uint64_t *const array_val)
+{
+	union bpf_attr elem_attr = {
+		.map_fd = map_fd,
+		.key = ptr_to_u64(array_indx),
+		.value = ptr_to_u64(array_val),
+		.flags = 0
+	};
+
+	TEST(bpf(BPF_MAP_LOOKUP_ELEM, &elem_attr, sizeof(elem_attr)));
+
+	if (TST_RET) {
+		tst_brk(TBROK | TTERRNO,
+			"Failed array map lookup: [%"PRIu32, *array_indx);
+	}
+
+	return TST_RET;
+}
+
 void bpf_init_prog_attr(union bpf_attr *attr, const struct bpf_insn *prog,
 	size_t prog_size, char *log_buf, size_t log_size)
 {
diff --git a/testcases/kernel/syscalls/bpf/bpf_common.h b/testcases/kernel/syscalls/bpf/bpf_common.h
index d36a2b09f..fb5731c3d 100644
--- a/testcases/kernel/syscalls/bpf/bpf_common.h
+++ b/testcases/kernel/syscalls/bpf/bpf_common.h
@@ -6,12 +6,22 @@
 #ifndef LTP_BPF_COMMON_H
 #define LTP_BPF_COMMON_H
 
+#include <sys/types.h>
+#include <inttypes.h>
+
+#include "lapi/bpf.h"
+
 #define BPF_MEMLOCK_ADD (2*1024*1024)
 
 void rlimit_bump_memlock(void);
 int bpf_map_create(union bpf_attr *attr);
+int bpf_map_array_create(uint32_t max_entries);
+long bpf_map_array_get(const int map_fd,
+		       const uint32_t *const array_indx,
+		       uint64_t *const array_val);
+
 void bpf_init_prog_attr(union bpf_attr *attr, const struct bpf_insn *prog,
-	size_t prog_size, char *log_buf, size_t log_size);
+			size_t prog_size, char *log_buf, size_t log_size);
 int bpf_load_prog(union bpf_attr *attr, const char *log);
 
 #endif
diff --git a/testcases/kernel/syscalls/bpf/bpf_prog01.c b/testcases/kernel/syscalls/bpf/bpf_prog01.c
index 966bf2092..ac57b24b5 100644
--- a/testcases/kernel/syscalls/bpf/bpf_prog01.c
+++ b/testcases/kernel/syscalls/bpf/bpf_prog01.c
@@ -83,14 +83,7 @@ void run(void)
 	uint32_t key = 0;
 	uint64_t val;
 
-	memset(attr, 0, sizeof(*attr));
-	attr->map_type = BPF_MAP_TYPE_ARRAY;
-	attr->key_size = 4;
-	attr->value_size = 8;
-	attr->max_entries = 1;
-
-	map_fd = bpf_map_create(attr);
-
+	map_fd = bpf_map_array_create(1);
 	prog_fd = load_prog(map_fd);
 
 	SAFE_SOCKETPAIR(AF_UNIX, SOCK_DGRAM, 0, sk);
@@ -99,15 +92,12 @@ void run(void)
 
 	SAFE_WRITE(1, sk[0], msg, sizeof(MSG));
 
-	memset(attr, 0, sizeof(*attr));
-	attr->map_fd = map_fd;
-	attr->key = ptr_to_u64(&key);
-	attr->value = ptr_to_u64(&val);
+	SAFE_CLOSE(prog_fd);
+	SAFE_CLOSE(sk[0]);
+	SAFE_CLOSE(sk[1]);
 
-	TEST(bpf(BPF_MAP_LOOKUP_ELEM, attr, sizeof(*attr)));
-	if (TST_RET == -1) {
-		tst_res(TFAIL | TTERRNO, "array map lookup");
-	} else if (val != 1) {
+        bpf_map_array_get(map_fd, &key, &val);
+	if (val != 1) {
 		tst_res(TFAIL,
 			"val = %lu, but should be val = 1",
 			val);
@@ -115,10 +105,7 @@ void run(void)
 	        tst_res(TPASS, "val = 1");
 	}
 
-	SAFE_CLOSE(prog_fd);
 	SAFE_CLOSE(map_fd);
-	SAFE_CLOSE(sk[0]);
-	SAFE_CLOSE(sk[1]);
 }
 
 static struct tst_test test = {
diff --git a/testcases/kernel/syscalls/bpf/bpf_prog02.c b/testcases/kernel/syscalls/bpf/bpf_prog02.c
index eeba16a54..4558153ea 100644
--- a/testcases/kernel/syscalls/bpf/bpf_prog02.c
+++ b/testcases/kernel/syscalls/bpf/bpf_prog02.c
@@ -81,14 +81,7 @@ static void run(void)
 	int map_fd, prog_fd;
 	int sk[2];
 
-	memset(attr, 0, sizeof(*attr));
-	attr->map_type = BPF_MAP_TYPE_ARRAY;
-	attr->key_size = 4;
-	attr->value_size = 8;
-	attr->max_entries = 2;
-
-	map_fd = bpf_map_create(attr);
-
+	map_fd = bpf_map_array_create(2);
 	prog_fd = load_prog(map_fd);
 
 	SAFE_SOCKETPAIR(AF_UNIX, SOCK_DGRAM, 0, sk);
@@ -97,49 +90,31 @@ static void run(void)
 
 	SAFE_WRITE(1, sk[0], msg, sizeof(MSG));
 
-	memset(attr, 0, sizeof(*attr));
-	attr->map_fd = map_fd;
-	attr->key = ptr_to_u64(key);
-	attr->value = ptr_to_u64(val);
-	*key = 0;
-
-	TEST(bpf(BPF_MAP_LOOKUP_ELEM, attr, sizeof(*attr)));
-	if (TST_RET == -1) {
-		tst_res(TFAIL | TTERRNO, "array map lookup");
-		goto exit;
-	}
+	SAFE_CLOSE(prog_fd);
+	SAFE_CLOSE(sk[0]);
+	SAFE_CLOSE(sk[1]);
 
+        *key = 0;
+	bpf_map_array_get(map_fd, key, val);
 	if (*val != A64INT + 1) {
 		tst_res(TFAIL,
 			"val = %"PRIu64", but should be val = %"PRIu64" + 1",
 			*val, A64INT);
-		goto exit;
+	} else {
+		tst_res(TPASS, "val = %"PRIu64" + 1", A64INT);
 	}
 
-	tst_res(TPASS, "val = %"PRIu64" + 1", A64INT);
-
 	*key = 1;
-
-	TEST(bpf(BPF_MAP_LOOKUP_ELEM, attr, sizeof(*attr)));
-	if (TST_RET == -1) {
-		tst_res(TFAIL | TTERRNO, "array map lookup");
-		goto exit;
-	}
-
+	bpf_map_array_get(map_fd, key, val);
 	if (*val != A64INT - 1) {
 		tst_res(TFAIL,
 			"val = %"PRIu64", but should be val = %"PRIu64" - 1",
 			*val, A64INT);
-		goto exit;
+	} else {
+		tst_res(TPASS, "val = %"PRIu64" - 1", A64INT);
 	}
 
-	tst_res(TPASS, "val = %"PRIu64" - 1", A64INT);
-
-exit:
-	SAFE_CLOSE(prog_fd);
 	SAFE_CLOSE(map_fd);
-	SAFE_CLOSE(sk[0]);
-	SAFE_CLOSE(sk[1]);
 }
 
 static struct tst_test test = {
diff --git a/testcases/kernel/syscalls/bpf/bpf_prog03.c b/testcases/kernel/syscalls/bpf/bpf_prog03.c
index 5b8a394e8..1195ddc2c 100644
--- a/testcases/kernel/syscalls/bpf/bpf_prog03.c
+++ b/testcases/kernel/syscalls/bpf/bpf_prog03.c
@@ -119,13 +119,7 @@ static void run(void)
 	int map_fd, prog_fd;
 	int sk[2];
 
-	memset(attr, 0, sizeof(*attr));
-	attr->map_type = BPF_MAP_TYPE_ARRAY;
-	attr->key_size = 4;
-	attr->value_size = 8;
-	attr->max_entries = 32;
-
-	map_fd = bpf_map_create(attr);
+	map_fd = bpf_map_array_create(32);
 
 	memset(attr, 0, sizeof(*attr));
 	attr->map_fd = map_fd;
@@ -148,22 +142,13 @@ static void run(void)
 			&prog_fd, sizeof(prog_fd));
 
 	SAFE_WRITE(1, sk[0], msg, sizeof(MSG));
-
-	memset(attr, 0, sizeof(*attr));
-	attr->map_fd = map_fd;
-	attr->key = ptr_to_u64(key);
-	attr->value = ptr_to_u64(val);
-	*key = 0;
-
-	TEST(bpf(BPF_MAP_LOOKUP_ELEM, attr, sizeof(*attr)));
-	if (TST_RET == -1)
-		tst_res(TFAIL | TTERRNO, "array map lookup");
-	else
-		tst_res(TINFO, "Pointer offset was 0x%"PRIx64, *val);
-
 	SAFE_CLOSE(sk[0]);
 	SAFE_CLOSE(sk[1]);
 	SAFE_CLOSE(prog_fd);
+
+	*key = 0;
+	bpf_map_array_get(map_fd, key, val);
+	tst_res(TINFO, "Pointer offset was 0x%"PRIx64, *val);
 exit:
 	SAFE_CLOSE(map_fd);
 }
diff --git a/testcases/kernel/syscalls/bpf/bpf_prog04.c b/testcases/kernel/syscalls/bpf/bpf_prog04.c
index 799fcb52d..1a1ee0f04 100644
--- a/testcases/kernel/syscalls/bpf/bpf_prog04.c
+++ b/testcases/kernel/syscalls/bpf/bpf_prog04.c
@@ -94,13 +94,7 @@ static void run(void)
 	int map_fd, prog_fd;
 	int sk[2];
 
-	memset(attr, 0, sizeof(*attr));
-	attr->map_type = BPF_MAP_TYPE_ARRAY;
-	attr->key_size = 4;
-	attr->value_size = 8;
-	attr->max_entries = 1;
-
-	map_fd = bpf_map_create(attr);
+	map_fd = bpf_map_array_create(1);
 	prog_fd = load_prog(map_fd);
 
 	if (prog_fd >= 0) {
-- 
2.31.1

[LTP] [PATCH v2 2/5] bpf: Add map_array helper functions

[Cyril Hrubis]

Hi!
> +long bpf_map_array_get(const int map_fd,
> +		       const uint32_t *const array_indx,
> +		       uint64_t *const array_val)

Wouldn't the API be nicer if we passed just index value and returned the
map value? We do TBROK if something goes wrong anyways, so there is no
point in passing the return value from the bpf() syscall.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v2 2/5] bpf: Add map_array helper functions

[Richard Palethorpe]

Hello,

Cyril Hrubis <chrubis@suse.cz> writes:

> Hi!
>> +long bpf_map_array_get(const int map_fd,
>> +		       const uint32_t *const array_indx,
>> +		       uint64_t *const array_val)
>
> Wouldn't the API be nicer if we passed just index value and returned the
> map value? We do TBROK if something goes wrong anyways, so there is no
> point in passing the return value from the bpf() syscall.

It would be nicer, but the existing tests put the key and value in
guarded buffers. I guess there are other types of memory we might want
to pass a pointer to as well.

-- 
Thank you,
Richard.

[LTP] [PATCH v2 3/5] bpf: Add helper to run socket programs

[Richard Palethorpe]

So far we always trigger the BPF programs the same way.

Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com>
---
 testcases/kernel/syscalls/bpf/bpf_common.c | 15 +++++++++++++++
 testcases/kernel/syscalls/bpf/bpf_common.h |  2 ++
 testcases/kernel/syscalls/bpf/bpf_prog01.c | 11 +----------
 testcases/kernel/syscalls/bpf/bpf_prog02.c | 11 +----------
 testcases/kernel/syscalls/bpf/bpf_prog03.c |  9 +--------
 testcases/kernel/syscalls/bpf/bpf_prog04.c |  9 +--------
 6 files changed, 21 insertions(+), 36 deletions(-)

diff --git a/testcases/kernel/syscalls/bpf/bpf_common.c b/testcases/kernel/syscalls/bpf/bpf_common.c
index 90772815b..b5337c22a 100644
--- a/testcases/kernel/syscalls/bpf/bpf_common.c
+++ b/testcases/kernel/syscalls/bpf/bpf_common.c
@@ -127,3 +127,18 @@ int bpf_load_prog(union bpf_attr *attr, const char *log)
 	tst_brk(TBROK | TERRNO, "Failed to load program");
 	return ret;
 }
+
+void bpf_run_prog(const int prog_fd,
+		  const char *const msg, const size_t msg_len)
+{
+	int sk[2];
+
+	SAFE_SOCKETPAIR(AF_UNIX, SOCK_DGRAM, 0, sk);
+	SAFE_SETSOCKOPT(sk[1], SOL_SOCKET, SO_ATTACH_BPF,
+			&prog_fd, sizeof(prog_fd));
+
+	SAFE_WRITE(1, sk[0], msg, msg_len);
+
+	SAFE_CLOSE(sk[0]);
+	SAFE_CLOSE(sk[1]);
+}
diff --git a/testcases/kernel/syscalls/bpf/bpf_common.h b/testcases/kernel/syscalls/bpf/bpf_common.h
index fb5731c3d..9e9935c2c 100644
--- a/testcases/kernel/syscalls/bpf/bpf_common.h
+++ b/testcases/kernel/syscalls/bpf/bpf_common.h
@@ -23,5 +23,7 @@ long bpf_map_array_get(const int map_fd,
 void bpf_init_prog_attr(union bpf_attr *attr, const struct bpf_insn *prog,
 			size_t prog_size, char *log_buf, size_t log_size);
 int bpf_load_prog(union bpf_attr *attr, const char *log);
+void bpf_run_prog(const int prog_fd,
+		  const char *const msg, const size_t msg_len);
 
 #endif
diff --git a/testcases/kernel/syscalls/bpf/bpf_prog01.c b/testcases/kernel/syscalls/bpf/bpf_prog01.c
index ac57b24b5..1d5d04556 100644
--- a/testcases/kernel/syscalls/bpf/bpf_prog01.c
+++ b/testcases/kernel/syscalls/bpf/bpf_prog01.c
@@ -79,22 +79,13 @@ void setup(void)
 void run(void)
 {
 	int map_fd, prog_fd;
-	int sk[2];
 	uint32_t key = 0;
 	uint64_t val;
 
 	map_fd = bpf_map_array_create(1);
 	prog_fd = load_prog(map_fd);
-
-	SAFE_SOCKETPAIR(AF_UNIX, SOCK_DGRAM, 0, sk);
-	SAFE_SETSOCKOPT(sk[1], SOL_SOCKET, SO_ATTACH_BPF,
-			&prog_fd, sizeof(prog_fd));
-
-	SAFE_WRITE(1, sk[0], msg, sizeof(MSG));
-
+	bpf_run_prog(prog_fd, msg, sizeof(MSG));
 	SAFE_CLOSE(prog_fd);
-	SAFE_CLOSE(sk[0]);
-	SAFE_CLOSE(sk[1]);
 
         bpf_map_array_get(map_fd, &key, &val);
 	if (val != 1) {
diff --git a/testcases/kernel/syscalls/bpf/bpf_prog02.c b/testcases/kernel/syscalls/bpf/bpf_prog02.c
index 4558153ea..9f6acca60 100644
--- a/testcases/kernel/syscalls/bpf/bpf_prog02.c
+++ b/testcases/kernel/syscalls/bpf/bpf_prog02.c
@@ -79,20 +79,11 @@ static void setup(void)
 static void run(void)
 {
 	int map_fd, prog_fd;
-	int sk[2];
 
 	map_fd = bpf_map_array_create(2);
 	prog_fd = load_prog(map_fd);
-
-	SAFE_SOCKETPAIR(AF_UNIX, SOCK_DGRAM, 0, sk);
-	SAFE_SETSOCKOPT(sk[1], SOL_SOCKET, SO_ATTACH_BPF,
-			&prog_fd, sizeof(prog_fd));
-
-	SAFE_WRITE(1, sk[0], msg, sizeof(MSG));
-
+	bpf_run_prog(prog_fd, msg, sizeof(MSG));
 	SAFE_CLOSE(prog_fd);
-	SAFE_CLOSE(sk[0]);
-	SAFE_CLOSE(sk[1]);
 
         *key = 0;
 	bpf_map_array_get(map_fd, key, val);
diff --git a/testcases/kernel/syscalls/bpf/bpf_prog03.c b/testcases/kernel/syscalls/bpf/bpf_prog03.c
index 1195ddc2c..9a7af7f4c 100644
--- a/testcases/kernel/syscalls/bpf/bpf_prog03.c
+++ b/testcases/kernel/syscalls/bpf/bpf_prog03.c
@@ -117,7 +117,6 @@ static void setup(void)
 static void run(void)
 {
 	int map_fd, prog_fd;
-	int sk[2];
 
 	map_fd = bpf_map_array_create(32);
 
@@ -137,13 +136,7 @@ static void run(void)
 
 	tst_res(TFAIL, "Loaded bad eBPF, now we will run it and maybe crash");
 
-	SAFE_SOCKETPAIR(AF_UNIX, SOCK_DGRAM, 0, sk);
-	SAFE_SETSOCKOPT(sk[1], SOL_SOCKET, SO_ATTACH_BPF,
-			&prog_fd, sizeof(prog_fd));
-
-	SAFE_WRITE(1, sk[0], msg, sizeof(MSG));
-	SAFE_CLOSE(sk[0]);
-	SAFE_CLOSE(sk[1]);
+	bpf_run_prog(prog_fd, msg, sizeof(MSG));
 	SAFE_CLOSE(prog_fd);
 
 	*key = 0;
diff --git a/testcases/kernel/syscalls/bpf/bpf_prog04.c b/testcases/kernel/syscalls/bpf/bpf_prog04.c
index 1a1ee0f04..09d0cc468 100644
--- a/testcases/kernel/syscalls/bpf/bpf_prog04.c
+++ b/testcases/kernel/syscalls/bpf/bpf_prog04.c
@@ -92,7 +92,6 @@ static void setup(void)
 static void run(void)
 {
 	int map_fd, prog_fd;
-	int sk[2];
 
 	map_fd = bpf_map_array_create(1);
 	prog_fd = load_prog(map_fd);
@@ -100,13 +99,7 @@ static void run(void)
 	if (prog_fd >= 0) {
 		tst_res(TFAIL, "Malicious eBPF code passed verification. "
 			"Now let's try crashing the kernel.");
-		SAFE_SOCKETPAIR(AF_UNIX, SOCK_DGRAM, 0, sk);
-		SAFE_SETSOCKOPT(sk[1], SOL_SOCKET, SO_ATTACH_BPF, &prog_fd,
-			sizeof(prog_fd));
-
-		SAFE_WRITE(1, sk[0], msg, sizeof(MSG));
-		SAFE_CLOSE(sk[0]);
-		SAFE_CLOSE(sk[1]);
+		bpf_run_prog(prog_fd, msg, sizeof(MSG));
 	}
 
 	if (prog_fd >= 0)
-- 
2.31.1

[LTP] [PATCH v2 3/5] bpf: Add helper to run socket programs

[Cyril Hrubis]

Hi!
Reviewed-by: Cyril Hrubis <chrubis@suse.cz>

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v2 4/5] bpf: Add bpf_insn_buf, map and instruction concatenation helpers

[Richard Palethorpe]

Add helpers for building up programs. Tests before bpf_prog05 have not
been updated to use this for two reasons.

1. Some apply offsets to the map pointer returned or jump to the end
   of the program instead of just over an immediate exit
   instruction. Either way modifying them would require testing they
   still reproduce the bug.

2. Some have a lot of comments describing the program which is useful
   to learn from. These would need to be moved.

Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com>
---
 testcases/kernel/syscalls/bpf/bpf_common.c | 40 ++++++++++++++++++++++
 testcases/kernel/syscalls/bpf/bpf_common.h | 12 +++++++
 2 files changed, 52 insertions(+)

diff --git a/testcases/kernel/syscalls/bpf/bpf_common.c b/testcases/kernel/syscalls/bpf/bpf_common.c
index b5337c22a..d80ed91bb 100644
--- a/testcases/kernel/syscalls/bpf/bpf_common.c
+++ b/testcases/kernel/syscalls/bpf/bpf_common.c
@@ -82,6 +82,46 @@ long bpf_map_array_get(const int map_fd,
 	return TST_RET;
 }
 
+void bpf_insn_buf_cat(struct bpf_insn_buf *const self,
+		      const struct bpf_insn *const insn_to_cat,
+		      const size_t insn_to_cat_len)
+{
+	memcpy(((char *)self->insn) + self->byte_len,
+	       insn_to_cat, insn_to_cat_len);
+	self->byte_len += insn_to_cat_len;
+}
+
+/* map[array_indx] = reg_to_save
+ *
+ * Inserts the following into insn_out.
+ *
+ * r1 = map_fd
+ * r2 = fp
+ * r2 = r2 - 4
+ * r2 = array_indx
+ * call map_lookup_elem(r1, r2)
+ * if r0 != 0 goto pc+1
+ * exit
+ * *r0 = reg_to_save
+ *
+ */
+ void bpf_insn_buf_array_set(struct bpf_insn_buf *const self, const int map_fd,
+			     const uint32_t array_indx, const uint8_t reg_to_save)
+{
+	const struct bpf_insn map_insn[] = {
+		BPF_LD_MAP_FD(BPF_REG_1, map_fd),
+		BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
+		BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -4),
+		BPF_ST_MEM(BPF_W, BPF_REG_2, 0, array_indx),
+		BPF_EMIT_CALL(BPF_FUNC_map_lookup_elem),
+		BPF_JMP_IMM(BPF_JNE, BPF_REG_0, 0, 1),
+		BPF_EXIT_INSN(),
+		BPF_STX_MEM(BPF_DW, BPF_REG_0, reg_to_save, 0),
+	};
+
+	bpf_insn_buf_cat(self, map_insn, sizeof(map_insn));
+}
+
 void bpf_init_prog_attr(union bpf_attr *attr, const struct bpf_insn *prog,
 	size_t prog_size, char *log_buf, size_t log_size)
 {
diff --git a/testcases/kernel/syscalls/bpf/bpf_common.h b/testcases/kernel/syscalls/bpf/bpf_common.h
index 9e9935c2c..10b1eee86 100644
--- a/testcases/kernel/syscalls/bpf/bpf_common.h
+++ b/testcases/kernel/syscalls/bpf/bpf_common.h
@@ -13,6 +13,11 @@
 
 #define BPF_MEMLOCK_ADD (2*1024*1024)
 
+struct bpf_insn_buf {
+	size_t byte_len;
+	struct bpf_insn insn[BPF_MAXINSNS];
+};
+
 void rlimit_bump_memlock(void);
 int bpf_map_create(union bpf_attr *attr);
 int bpf_map_array_create(uint32_t max_entries);
@@ -20,6 +25,13 @@ long bpf_map_array_get(const int map_fd,
 		       const uint32_t *const array_indx,
 		       uint64_t *const array_val);
 
+void bpf_insn_buf_cat(struct bpf_insn_buf *const self,
+		      const struct bpf_insn *const insn_to_cat,
+		      const size_t insn_to_cat_len);
+void bpf_insn_buf_array_set(struct bpf_insn_buf *const self,
+			    const int map_fd,
+			    const uint32_t array_indx, const uint8_t reg_to_save);
+
 void bpf_init_prog_attr(union bpf_attr *attr, const struct bpf_insn *prog,
 			size_t prog_size, char *log_buf, size_t log_size);
 int bpf_load_prog(union bpf_attr *attr, const char *log);
-- 
2.31.1

[LTP] [PATCH v2 4/5] bpf: Add bpf_insn_buf, map and instruction concatenation helpers

[Cyril Hrubis]

Hi!
> Add helpers for building up programs. Tests before bpf_prog05 have not
> been updated to use this for two reasons.
> 
> 1. Some apply offsets to the map pointer returned or jump to the end
>    of the program instead of just over an immediate exit
>    instruction. Either way modifying them would require testing they
>    still reproduce the bug.
> 
> 2. Some have a lot of comments describing the program which is useful
>    to learn from. These would need to be moved.
> 
> Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com>
> ---
>  testcases/kernel/syscalls/bpf/bpf_common.c | 40 ++++++++++++++++++++++
>  testcases/kernel/syscalls/bpf/bpf_common.h | 12 +++++++
>  2 files changed, 52 insertions(+)
> 
> diff --git a/testcases/kernel/syscalls/bpf/bpf_common.c b/testcases/kernel/syscalls/bpf/bpf_common.c
> index b5337c22a..d80ed91bb 100644
> --- a/testcases/kernel/syscalls/bpf/bpf_common.c
> +++ b/testcases/kernel/syscalls/bpf/bpf_common.c
> @@ -82,6 +82,46 @@ long bpf_map_array_get(const int map_fd,
>  	return TST_RET;
>  }
>  
> +void bpf_insn_buf_cat(struct bpf_insn_buf *const self,
> +		      const struct bpf_insn *const insn_to_cat,
> +		      const size_t insn_to_cat_len)
> +{
> +	memcpy(((char *)self->insn) + self->byte_len,
> +	       insn_to_cat, insn_to_cat_len);
> +	self->byte_len += insn_to_cat_len;
> +}
> +
> +/* map[array_indx] = reg_to_save
> + *
> + * Inserts the following into insn_out.
> + *
> + * r1 = map_fd
> + * r2 = fp
> + * r2 = r2 - 4
> + * r2 = array_indx
> + * call map_lookup_elem(r1, r2)
> + * if r0 != 0 goto pc+1
> + * exit
> + * *r0 = reg_to_save
> + *
> + */
> + void bpf_insn_buf_array_set(struct bpf_insn_buf *const self, const int map_fd,
> +			     const uint32_t array_indx, const uint8_t reg_to_save)
> +{
> +	const struct bpf_insn map_insn[] = {
> +		BPF_LD_MAP_FD(BPF_REG_1, map_fd),
> +		BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
> +		BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -4),
> +		BPF_ST_MEM(BPF_W, BPF_REG_2, 0, array_indx),
> +		BPF_EMIT_CALL(BPF_FUNC_map_lookup_elem),
> +		BPF_JMP_IMM(BPF_JNE, BPF_REG_0, 0, 1),
> +		BPF_EXIT_INSN(),
> +		BPF_STX_MEM(BPF_DW, BPF_REG_0, reg_to_save, 0),
> +	};

I'm wondering if it would be easier to write a macro that would produce
this code, something as:

	#define BPF_ARRAY_STORE(map_fd, arr_idx, reg_to_save) \
		BPF_LD_MAP_FD(BPF_REG_1, map_fd), \
		BPF_MOV64_REG(BPF_REG_2, BPF_REG_10), \
		BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -4), \
		BPF_ST_MEM(BPF_W, BPF_REG_2, 0, arr_indx), \
		BPF_EMIT_CALL(BPF_FUNC_map_lookup_elem), \
		BPF_JMP_IMM(BPF_JNE, BPF_REG_0, 0, 1), \
		BPF_EXIT_INSN(), \
		BPF_STX_MEM(BPF_DW, BPF_REG_0, reg_to_save, 0)


Then we can use this piece as any other macros when constructing the
program, the only difference is that this macro emits 8 instructions
instead of one.

What do you think?

> +	bpf_insn_buf_cat(self, map_insn, sizeof(map_insn));
> +}
> +
>  void bpf_init_prog_attr(union bpf_attr *attr, const struct bpf_insn *prog,
>  	size_t prog_size, char *log_buf, size_t log_size)
>  {
> diff --git a/testcases/kernel/syscalls/bpf/bpf_common.h b/testcases/kernel/syscalls/bpf/bpf_common.h
> index 9e9935c2c..10b1eee86 100644
> --- a/testcases/kernel/syscalls/bpf/bpf_common.h
> +++ b/testcases/kernel/syscalls/bpf/bpf_common.h
> @@ -13,6 +13,11 @@
>  
>  #define BPF_MEMLOCK_ADD (2*1024*1024)
>  
> +struct bpf_insn_buf {
> +	size_t byte_len;
> +	struct bpf_insn insn[BPF_MAXINSNS];
> +};
> +
>  void rlimit_bump_memlock(void);
>  int bpf_map_create(union bpf_attr *attr);
>  int bpf_map_array_create(uint32_t max_entries);
> @@ -20,6 +25,13 @@ long bpf_map_array_get(const int map_fd,
>  		       const uint32_t *const array_indx,
>  		       uint64_t *const array_val);
>  
> +void bpf_insn_buf_cat(struct bpf_insn_buf *const self,
> +		      const struct bpf_insn *const insn_to_cat,
> +		      const size_t insn_to_cat_len);
> +void bpf_insn_buf_array_set(struct bpf_insn_buf *const self,
> +			    const int map_fd,
> +			    const uint32_t array_indx, const uint8_t reg_to_save);
> +
>  void bpf_init_prog_attr(union bpf_attr *attr, const struct bpf_insn *prog,
>  			size_t prog_size, char *log_buf, size_t log_size);
>  int bpf_load_prog(union bpf_attr *attr, const char *log);
> -- 
> 2.31.1
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v2 5/5] bpf: Check truncation on 32bit div/mod by zero

[Richard Palethorpe]

Add a test which checks for a number of issues surrounding division by
zero.

Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com>
---
 runtest/cve                                |   1 +
 runtest/syscalls                           |   1 +
 testcases/kernel/syscalls/bpf/.gitignore   |   1 +
 testcases/kernel/syscalls/bpf/bpf_prog05.c | 175 +++++++++++++++++++++
 4 files changed, 178 insertions(+)
 create mode 100644 testcases/kernel/syscalls/bpf/bpf_prog05.c

diff --git a/runtest/cve b/runtest/cve
index f650854f9..3beb88bb0 100644
--- a/runtest/cve
+++ b/runtest/cve
@@ -61,3 +61,4 @@ cve-2020-11494 pty04
 cve-2020-14386 sendto03
 cve-2020-14416 pty03
 cve-2020-29373 io_uring02
+cve-2021-3444 bpf_prog05
diff --git a/runtest/syscalls b/runtest/syscalls
index 546a988c2..60c0a7a99 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -42,6 +42,7 @@ bpf_prog01 bpf_prog01
 bpf_prog02 bpf_prog02
 bpf_prog03 bpf_prog03
 bpf_prog04 bpf_prog04
+bpf_prog05 bpf_prog05
 
 brk01 brk01
 brk02 brk02
diff --git a/testcases/kernel/syscalls/bpf/.gitignore b/testcases/kernel/syscalls/bpf/.gitignore
index 74742c0cd..42365cef5 100644
--- a/testcases/kernel/syscalls/bpf/.gitignore
+++ b/testcases/kernel/syscalls/bpf/.gitignore
@@ -3,3 +3,4 @@ bpf_prog01
 bpf_prog02
 bpf_prog03
 bpf_prog04
+bpf_prog05
diff --git a/testcases/kernel/syscalls/bpf/bpf_prog05.c b/testcases/kernel/syscalls/bpf/bpf_prog05.c
new file mode 100644
index 000000000..ccaa2cbe1
--- /dev/null
+++ b/testcases/kernel/syscalls/bpf/bpf_prog05.c
@@ -0,0 +1,175 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2021 SUSE LLC <rpalethorpe@suse.com>
+ */
+
+/*\
+ * [Description]
+ *
+ * Compare the effects of 32-bit div/mod by zero with the "expected"
+ * behaviour.
+ *
+ * The commit "bpf: fix subprog verifier bypass by div/mod by 0
+ * exception", changed div/mod by zero from exiting the current
+ * program to setting the destination register to zero (div) or
+ * leaving it untouched (mod).
+ *
+ * This solved one verfier bug which allowed dodgy pointer values, but
+ * it turned out that the source register was being 32-bit truncated
+ * when it should not be. Also the destination register for mod was
+ * not being truncated when it should be.
+ *
+ * So then we have the following two fixes:
+ * "bpf: Fix 32 bit src register truncation on div/mod"
+ * "bpf: Fix truncation handling for mod32 dst reg wrt zero"
+ *
+ * Testing for all of these issues is a problem. Not least because
+ * division by zero is undefined, so in theory any result is
+ * acceptable so long as the verifier and runtime behaviour
+ * match.
+ *
+ * However to keep things simple we just check if the source and
+ * destination register runtime values match the current upstream
+ * behaviour at the time of writing.
+ *
+ * If the test fails you may have one or more of the above patches
+ * missing. In this case it is possible that you are not vulnerable
+ * depending on what other backports and fixes have been applied. If
+ * upstream changes the behaviour of division by zero, then the test
+ * will need updating.
+ *
+ * Note that we use r6 as the src register and r7 as the dst. w6 and
+ * w7 are the same registers treated as 32bit.
+ */
+
+#include <stdio.h>
+#include <string.h>
+#include <inttypes.h>
+
+#include "config.h"
+#include "tst_test.h"
+#include "tst_taint.h"
+#include "tst_capability.h"
+#include "lapi/socket.h"
+#include "lapi/bpf.h"
+#include "bpf_common.h"
+
+#define BUFSIZE 8192
+
+static const char MSG[] = "Ahoj!";
+static char *msg;
+
+static int map_fd;
+static uint32_t *key;
+static uint64_t *val;
+static char *log;
+static union bpf_attr *attr;
+
+static int load_prog(void)
+{
+	struct bpf_insn_buf prog_insn = { 0 };
+        /* r6 = 1 << 32
+	 * r7 = -1
+	 */
+	const struct bpf_insn set_src_dst_insn[] = {
+		BPF_LD_IMM64(BPF_REG_6, 1ULL << 32),
+		BPF_MOV64_IMM(BPF_REG_7, -1LL),
+	};
+	/* w7 /= w6 */
+	const struct bpf_insn div_insn =
+		BPF_ALU32_REG(BPF_DIV, BPF_REG_7, BPF_REG_6);
+	/* w7 %= w6 */
+	const struct bpf_insn mod_insn =
+		BPF_ALU32_REG(BPF_MOD, BPF_REG_7, BPF_REG_6);
+	/* exit(0) */
+	const struct bpf_insn exit_insn[] = {
+		BPF_MOV64_IMM(BPF_REG_0, 0),
+		BPF_EXIT_INSN()
+	};
+
+	bpf_insn_buf_cat(&prog_insn, set_src_dst_insn, sizeof(set_src_dst_insn));
+	bpf_insn_buf_cat(&prog_insn, &div_insn, sizeof(div_insn));
+
+	bpf_insn_buf_array_set(&prog_insn, map_fd, 0, BPF_REG_6);
+	bpf_insn_buf_array_set(&prog_insn, map_fd, 1, BPF_REG_7);
+
+	bpf_insn_buf_cat(&prog_insn, set_src_dst_insn, sizeof(set_src_dst_insn));
+	bpf_insn_buf_cat(&prog_insn, &mod_insn, sizeof(mod_insn));
+
+	bpf_insn_buf_array_set(&prog_insn, map_fd, 2, BPF_REG_6);
+	bpf_insn_buf_array_set(&prog_insn, map_fd, 3, BPF_REG_7);
+
+	bpf_insn_buf_cat(&prog_insn, exit_insn, sizeof(exit_insn));
+
+        bpf_init_prog_attr(attr, prog_insn.insn, prog_insn.byte_len, log, BUFSIZE);
+
+	return bpf_load_prog(attr, log);
+}
+
+static void expect_reg_val(const char *const reg_name,
+			   const uint64_t expected_val)
+{
+        bpf_map_array_get(map_fd, key, val);
+        (*key)++;
+
+        if (*val != expected_val) {
+		tst_res(TFAIL,
+			"%s = %"PRIu64", but should be %"PRIu64,
+			reg_name, *val, expected_val);
+	} else {
+		tst_res(TPASS, "%s = %"PRIu64, reg_name, *val);
+	}
+}
+
+static void setup(void)
+{
+	rlimit_bump_memlock();
+	memcpy(msg, MSG, sizeof(MSG));
+}
+
+static void run(void)
+{
+	int prog_fd;
+
+	map_fd = bpf_map_array_create(4);
+	prog_fd = load_prog();
+	bpf_run_prog(prog_fd, msg, sizeof(MSG));
+	SAFE_CLOSE(prog_fd);
+
+	tst_res(TINFO, "Check w7(-1) /= w6(0) [r7 = -1, r6 = 1 << 32]");
+	expect_reg_val("src(r6)", 1UL << 32);
+	expect_reg_val("dst(r7)", 0);
+
+	tst_res(TINFO, "Check w7(-1) %%= w6(0) [r7 = -1, r6 = 1 << 32]");
+	expect_reg_val("src(r6)", 1UL << 32);
+	expect_reg_val("dst(r7)", (uint32_t)-1);
+
+	SAFE_CLOSE(map_fd);
+}
+
+static struct tst_test test = {
+	.setup = setup,
+	.test_all = run,
+	.min_kver = "3.18",
+	.taint_check = TST_TAINT_W | TST_TAINT_D,
+	.caps = (struct tst_cap []) {
+		TST_CAP(TST_CAP_DROP, CAP_SYS_ADMIN),
+		{}
+	},
+	.bufs = (struct tst_buffers []) {
+		{&key, .size = sizeof(*key)},
+		{&val, .size = sizeof(*val)},
+		{&log, .size = BUFSIZE},
+		{&attr, .size = sizeof(*attr)},
+		{&msg, .size = sizeof(MSG)},
+		{}
+	},
+	.tags = (const struct tst_tag[]) {
+		{"linux-git", "f6b1b3bf0d5f"},
+		{"linux-git", "468f6eafa6c4"},
+		{"linux-git", "e88b2c6e5a4d"},
+		{"linux-git", "9b00f1b78809"},
+		{"CVE", "CVE-2021-3444"},
+		{}
+	}
+};
-- 
2.31.1

[LTP] [PATCH v2 5/5] bpf: Check truncation on 32bit div/mod by zero

[Cyril Hrubis]

Hi!
> +static int load_prog(void)
> +{
> +	struct bpf_insn_buf prog_insn = { 0 };
> +        /* r6 = 1 << 32
> +	 * r7 = -1
> +	 */
> +	const struct bpf_insn set_src_dst_insn[] = {
> +		BPF_LD_IMM64(BPF_REG_6, 1ULL << 32),
> +		BPF_MOV64_IMM(BPF_REG_7, -1LL),
> +	};
> +	/* w7 /= w6 */
> +	const struct bpf_insn div_insn =
> +		BPF_ALU32_REG(BPF_DIV, BPF_REG_7, BPF_REG_6);
> +	/* w7 %= w6 */
> +	const struct bpf_insn mod_insn =
> +		BPF_ALU32_REG(BPF_MOD, BPF_REG_7, BPF_REG_6);
> +	/* exit(0) */
> +	const struct bpf_insn exit_insn[] = {
> +		BPF_MOV64_IMM(BPF_REG_0, 0),
> +		BPF_EXIT_INSN()
> +	};
> +
> +	bpf_insn_buf_cat(&prog_insn, set_src_dst_insn, sizeof(set_src_dst_insn));
> +	bpf_insn_buf_cat(&prog_insn, &div_insn, sizeof(div_insn));
> +
> +	bpf_insn_buf_array_set(&prog_insn, map_fd, 0, BPF_REG_6);
> +	bpf_insn_buf_array_set(&prog_insn, map_fd, 1, BPF_REG_7);
> +
> +	bpf_insn_buf_cat(&prog_insn, set_src_dst_insn, sizeof(set_src_dst_insn));
> +	bpf_insn_buf_cat(&prog_insn, &mod_insn, sizeof(mod_insn));
> +
> +	bpf_insn_buf_array_set(&prog_insn, map_fd, 2, BPF_REG_6);
> +	bpf_insn_buf_array_set(&prog_insn, map_fd, 3, BPF_REG_7);
> +
> +	bpf_insn_buf_cat(&prog_insn, exit_insn, sizeof(exit_insn));
> +
> +        bpf_init_prog_attr(attr, prog_insn.insn, prog_insn.byte_len, log, BUFSIZE);

I find this completely unreadable to be honest, the array with
instructions is much better.

-- 
Cyril Hrubis
chrubis@suse.cz