[PATCH bpf v5 2/2] selftests/bpf: Cover negative buffer pointer offsets

[Sun Jian <sun.jian.kdev@gmail.com>]

Add verifier coverage for constant negative offsets on PTR_TO_TP_BUFFER
and PTR_TO_BUF pointers. Both programs adjust the buffer pointer by -8
and access it at offset zero, so the negative effective start must be
rejected at load time.

Switch the raw tracepoint writable attach checks from nbd_send_request
to bpf_testmod_test_writable_bare_tp, avoiding a dependency on the NBD
tracepoint. Keep the existing past-end case and add a case with a
negative var_off compensated by a positive instruction offset. The
effective start remains non-negative, so the program loads, but its
access end exceeds the writable context size and
bpf_raw_tracepoint_open() must return -EINVAL.

Reported-by: Shung-Hsi Yu <shung-hsi.yu@suse.com>
Link: https://lore.kernel.org/bpf/alRtilWhKw4zzMkI@u94a
Cc: stable@vger.kernel.org # 5.2.0
Signed-off-by: Sun Jian <sun.jian.kdev@gmail.com>
---
 .../raw_tp_writable_reject_bad_access.c       | 57 +++++++++++++++++++
 .../raw_tp_writable_reject_nbd_invalid.c      | 43 --------------
 .../selftests/bpf/prog_tests/verifier.c       |  2 +
 .../selftests/bpf/progs/verifier_ptr_to_buf.c | 27 +++++++++
 .../bpf/progs/verifier_raw_tp_writable.c      | 16 ++++++
 5 files changed, 102 insertions(+), 43 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/raw_tp_writable_reject_bad_access.c
 delete mode 100644 tools/testing/selftests/bpf/prog_tests/raw_tp_writable_reject_nbd_invalid.c
 create mode 100644 tools/testing/selftests/bpf/progs/verifier_ptr_to_buf.c

diff --git a/tools/testing/selftests/bpf/prog_tests/raw_tp_writable_reject_bad_access.c b/tools/testing/selftests/bpf/prog_tests/raw_tp_writable_reject_bad_access.c
new file mode 100644
index 000000000000..b8538fc4fc3f
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/raw_tp_writable_reject_bad_access.c
@@ -0,0 +1,57 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <test_progs.h>
+#include "test_kmods/bpf_testmod.h"
+#include "bpf_util.h"
+
+static void check_attach_reject(const struct bpf_insn *program, size_t prog_len)
+{
+	LIBBPF_OPTS(bpf_prog_load_opts, opts);
+	char error[4096];
+	int bpf_fd, tp_fd;
+
+	opts.log_level = 2;
+	opts.log_buf = error;
+	opts.log_size = sizeof(error);
+
+	bpf_fd = bpf_prog_load(BPF_PROG_TYPE_RAW_TRACEPOINT_WRITABLE, NULL, "GPL v2",
+			       program, prog_len, &opts);
+	if (!ASSERT_GE(bpf_fd, 0, "prog_load"))
+		return;
+
+	tp_fd = bpf_raw_tracepoint_open("bpf_testmod_test_writable_bare_tp", bpf_fd);
+	ASSERT_EQ(tp_fd, -EINVAL, "bpf_raw_tracepoint_open");
+	if (tp_fd >= 0)
+		close(tp_fd);
+
+	close(bpf_fd);
+}
+
+void test_raw_tp_writable_reject_bad_access(void)
+{
+	const struct bpf_insn program[] = {
+		/* r6 is our tp buffer */
+		BPF_LDX_MEM(BPF_DW, BPF_REG_6, BPF_REG_1, 0),
+		/* one byte beyond the end of the writable context */
+		BPF_LDX_MEM(BPF_B, BPF_REG_0, BPF_REG_6,
+			    sizeof(struct bpf_testmod_test_writable_ctx)),
+		BPF_EXIT_INSN(),
+	};
+
+	const struct bpf_insn negative_var_off_program[] = {
+		BPF_LDX_MEM(BPF_DW, BPF_REG_6, BPF_REG_1, 0),
+		/* make var_off negative, but keep the effective access offset non-negative */
+		BPF_ALU64_IMM(BPF_ADD, BPF_REG_6, -8),
+		/* one byte beyond the end of the writable context */
+		BPF_LDX_MEM(BPF_B, BPF_REG_0, BPF_REG_6,
+			    sizeof(struct bpf_testmod_test_writable_ctx) + 8),
+		BPF_EXIT_INSN(),
+	};
+
+	if (test__start_subtest("past_end"))
+		check_attach_reject(program, ARRAY_SIZE(program));
+
+	if (test__start_subtest("negative_var_off_past_end"))
+		check_attach_reject(negative_var_off_program,
+				    ARRAY_SIZE(negative_var_off_program));
+}
diff --git a/tools/testing/selftests/bpf/prog_tests/raw_tp_writable_reject_nbd_invalid.c b/tools/testing/selftests/bpf/prog_tests/raw_tp_writable_reject_nbd_invalid.c
deleted file mode 100644
index 216b0dfac0fe..000000000000
--- a/tools/testing/selftests/bpf/prog_tests/raw_tp_writable_reject_nbd_invalid.c
+++ /dev/null
@@ -1,43 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-
-#include <test_progs.h>
-#include <linux/nbd.h>
-#include "bpf_util.h"
-
-void test_raw_tp_writable_reject_nbd_invalid(void)
-{
-	__u32 duration = 0;
-	char error[4096];
-	int bpf_fd = -1, tp_fd = -1;
-
-	const struct bpf_insn program[] = {
-		/* r6 is our tp buffer */
-		BPF_LDX_MEM(BPF_DW, BPF_REG_6, BPF_REG_1, 0),
-		/* one byte beyond the end of the nbd_request struct */
-		BPF_LDX_MEM(BPF_B, BPF_REG_0, BPF_REG_6,
-			    sizeof(struct nbd_request)),
-		BPF_EXIT_INSN(),
-	};
-
-	LIBBPF_OPTS(bpf_prog_load_opts, opts,
-		.log_level = 2,
-		.log_buf = error,
-		.log_size = sizeof(error),
-	);
-
-	bpf_fd = bpf_prog_load(BPF_PROG_TYPE_RAW_TRACEPOINT_WRITABLE, NULL, "GPL v2",
-			       program, ARRAY_SIZE(program),
-			       &opts);
-	if (CHECK(bpf_fd < 0, "bpf_raw_tracepoint_writable load",
-		  "failed: %d errno %d\n", bpf_fd, errno))
-		return;
-
-	tp_fd = bpf_raw_tracepoint_open("nbd_send_request", bpf_fd);
-	if (CHECK(tp_fd >= 0, "bpf_raw_tracepoint_writable open",
-		  "erroneously succeeded\n"))
-		goto out_bpffd;
-
-	close(tp_fd);
-out_bpffd:
-	close(bpf_fd);
-}
diff --git a/tools/testing/selftests/bpf/prog_tests/verifier.c b/tools/testing/selftests/bpf/prog_tests/verifier.c
index 8a3d69e2453c..be97f6887f0e 100644
--- a/tools/testing/selftests/bpf/prog_tests/verifier.c
+++ b/tools/testing/selftests/bpf/prog_tests/verifier.c
@@ -78,6 +78,7 @@
 #include "verifier_precision.skel.h"
 #include "verifier_prevent_map_lookup.skel.h"
 #include "verifier_private_stack.skel.h"
+#include "verifier_ptr_to_buf.skel.h"
 #include "verifier_raw_stack.skel.h"
 #include "verifier_raw_tp_writable.skel.h"
 #include "verifier_reg_equal.skel.h"
@@ -230,6 +231,7 @@ void test_verifier_or_jmp32_k(void)           { RUN(verifier_or_jmp32_k); }
 void test_verifier_precision(void)            { RUN(verifier_precision); }
 void test_verifier_prevent_map_lookup(void)   { RUN(verifier_prevent_map_lookup); }
 void test_verifier_private_stack(void)        { RUN(verifier_private_stack); }
+void test_verifier_ptr_to_buf(void)           { RUN(verifier_ptr_to_buf); }
 void test_verifier_raw_stack(void)            { RUN(verifier_raw_stack); }
 void test_verifier_raw_tp_writable(void)      { RUN(verifier_raw_tp_writable); }
 void test_verifier_reg_equal(void)            { RUN(verifier_reg_equal); }
diff --git a/tools/testing/selftests/bpf/progs/verifier_ptr_to_buf.c b/tools/testing/selftests/bpf/progs/verifier_ptr_to_buf.c
new file mode 100644
index 000000000000..12cf24db46a0
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/verifier_ptr_to_buf.c
@@ -0,0 +1,27 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <vmlinux.h>
+#include <bpf/bpf_helpers.h>
+#include "bpf_misc.h"
+
+SEC("iter/bpf_map_elem")
+__description("PTR_TO_BUF: reject negative const offset")
+__failure
+__msg("invalid negative rdwr buffer offset")
+__naked void ptr_to_buf_reject_negative_const_offset(void)
+{
+	asm volatile ("r0 = 0;					\
+	 r2 = *(u64 *)(r1 + %[value_off]);			\
+	 if r2 == 0 goto l0_%=;					\
+	 r2 += -8;						\
+	 r0 = *(u64 *)(r2 + 0);					\
+l0_%=:								\
+	 exit;							\
+	"
+	:
+	: __imm_const(value_off,
+		      offsetof(struct bpf_iter__bpf_map_elem, value))
+	: __clobber_all);
+}
+
+char _license[] SEC("license") = "GPL";
diff --git a/tools/testing/selftests/bpf/progs/verifier_raw_tp_writable.c b/tools/testing/selftests/bpf/progs/verifier_raw_tp_writable.c
index 14a0172e2141..4055a6443bc2 100644
--- a/tools/testing/selftests/bpf/progs/verifier_raw_tp_writable.c
+++ b/tools/testing/selftests/bpf/progs/verifier_raw_tp_writable.c
@@ -47,4 +47,20 @@ l0_%=:	/* shift the buffer pointer to a variable location */\
 	: __clobber_all);
 }
 
+SEC("raw_tracepoint.w")
+__description("raw_tracepoint_writable: reject negative const offset")
+__failure
+__msg("invalid negative tracepoint buffer offset")
+__naked void tracepoint_writable_reject_negative_const_offset(void)
+{
+	asm volatile ("					\
+	r6 = *(u64 *)(r1 + 0);				\
+	r6 += -8;					\
+	r0 = *(u64 *)(r6 + 0);				\
+	exit;						\
+"	:
+	:
+	: __clobber_all);
+}
+
 char _license[] SEC("license") = "GPL";
-- 
2.43.0