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));
 }

Reply via email to