Re: [PATCH bpf v4 1/2] bpf: Reject negative const offsets for buffer pointers

[Shung-Hsi Yu <shung-hsi.yu@suse.com>]

On Fri, Jul 10, 2026 at 06:27:41PM +0800, sun jian wrote:
[...]
> One detail about the attach-time check: I agree that some negative-offset
> constructions can still be rejected later if the max_tp_access accounting
> produces a large value. But the runtime reproducer I used exercises this
> specific access:
> 
>     r6 = *(u64 *)(r1 + 0)
>     r6 += -8
>     *(u64 *)(r6 + 0) = 0
> 
> Here reg->var_off.value is (u64)-8, off is 0, and size is 8. With the current
> accounting,
>     reg->var_off.value + off + size
> wraps to 0. So max_tp_access records 0 and the attach-time writable_size check
> does not reject it, even though the effective access starts at -8.

Yes, that should had been rejected.

> On the bpf base without the verifier fix, the temporary reproducer did load
> and attach successfully:
>     negative_bpf_load: PASS
>     negative_raw_tp_open: PASS
>     negative_test_run: PASS
> and KASAN reported the stack-out-of-bounds write in ___bpf_prog_run.
> 
> So for this particular range, this looks like more than a load-time policy
> difference: the missing lower-bound check leaves the access executable on the
> bpf tree.

Actually I got this reproduced in the *bpf-next* tree now. And I think I
know why Eduard wasn't able to reproduce the issue.

The main problem was that CONFIG_BLK_DEV_NBD was not enabled in the
default BPF selftests configuration, so nbd_send_request wasn't
available, and thus the attachment always fail with -2 without
exercising the checks that we have in bpf_probe_register().

With CONFIG_BLK_DEV_NBD enabled, and negative_var_off_program[] updated
to do use offset of 0 (instead of 28 + 8), I was able to reproduce the
issue on top of commit `9a3a07d06e7d` "Merge branch
'bpf-fix-warning-in-bpf_trampoline_multi_detach'" with the following
updated version of your patch.

  0: R1=ctx() R10=fp0
  0: (79) r6 = *(u64 *)(r1 +0)          ; R1=ctx() R6=tp_buffer()
  1: (07) r6 += -8                      ; R6=tp_buffer(imm=-8)
  2: (79) r0 = *(u64 *)(r6 +0)          ; R0=scalar() R6=tp_buffer(imm=-8)
  3: (95) exit
  processed 4 insns (limit 1000000) max_states_per_insn 0 total_states 0 peak_states 0 mark_read 0
  tp_fd: 8
  check_nbd_attach_reject:FAIL:nbd_invalid_negative_var_off unexpected nbd_invalid_negative_var_off: actual 8 >= expected 0
  #319     raw_tp_writable_reject_nbd_invalid:FAIL

So I would say a fixes tag that points to 022ac0750883 is still needed
after all.

---

diff --git a/tools/testing/selftests/bpf/config b/tools/testing/selftests/bpf/config
index adb25146e88c..e1797bd87904 100644
--- a/tools/testing/selftests/bpf/config
+++ b/tools/testing/selftests/bpf/config
@@ -1,4 +1,5 @@
 CONFIG_BLK_DEV_LOOP=y
+CONFIG_BLK_DEV_NBD=y
 CONFIG_BOOTPARAM_HARDLOCKUP_PANIC=y
 CONFIG_BOOTPARAM_SOFTLOCKUP_PANIC=1
 CONFIG_BPF=y
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
index 216b0dfac0fe..0ed1d560a2f4 100644
--- 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
@@ -4,12 +4,34 @@
 #include <linux/nbd.h>
 #include "bpf_util.h"
 
+static void check_nbd_attach_reject(const char *name,
+				    const struct bpf_insn *program, size_t prog_len)
+{
+	LIBBPF_OPTS(bpf_prog_load_opts, opts);
+	char error[8192];
+	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;
+
+	printf("verifier log: %s", error);
+
+	tp_fd = bpf_raw_tracepoint_open("nbd_send_request", bpf_fd);
+	printf("tp_fd: %d\n", tp_fd);
+	if (!ASSERT_LT(tp_fd, 0, name))
+		close(tp_fd);
+
+	close(bpf_fd);
+}
+
 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),
@@ -19,25 +41,17 @@ void test_raw_tp_writable_reject_nbd_invalid(void)
 		BPF_EXIT_INSN(),
 	};
 
-	LIBBPF_OPTS(bpf_prog_load_opts, opts,
-		.log_level = 2,
-		.log_buf = error,
-		.log_size = sizeof(error),
-	);
+	const struct bpf_insn negative_var_off_program[] = {
+		/* r6 is our tp buffer */
+		BPF_LDX_MEM(BPF_DW, BPF_REG_6, BPF_REG_1, 0),
+		/* eight bytes before the start of the nbd_request struct */
+		BPF_ALU64_IMM(BPF_ADD, BPF_REG_6, -8),
+		BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_6, 0),
+		BPF_EXIT_INSN(),
+	};
 
-	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);
+	check_nbd_attach_reject("nbd_invalid", program, ARRAY_SIZE(program));
+	check_nbd_attach_reject("nbd_invalid_negative_var_off",
+				negative_var_off_program,
+				ARRAY_SIZE(negative_var_off_program));
 }