On Mon, 2026-07-13 at 13:05 +0800, Shung-Hsi Yu wrote:
> 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.
Hi Shung-Hsi, Sun,
Shung-Hsi, thank you for figuring out the quirk with the config.
Indeed if I modify the test to be config independent the following
program is both accepted at load and attached:
*r6 = *(u64 *)(r1 +0)
r6 += -8
r0 = *(u64 *)(r6 +0)
Worse yet, the mechanism affects both PTR_TO_TP_BUFFER and PTR_TO_BUF.
The PTR_TO_BUF is reachable from helpers/kfuncs with custom 'size' assignments,
as far as I understand.
Sun, of checks added by this patch:
> var_off = (s64)reg->var_off.value;
> if (var_off >= BPF_MAX_VAR_OFF || var_off <= -BPF_MAX_VAR_OFF)
This should hold already.
> start = var_off + off;
> if (start < 0)
This one is necessary.
> if (size < 0) {
Technically this one is bounded by 1,2,4,8 in case of instructions and
by BPF_MAX_VAR_SIZ in case of a helper/kfunc (see check_mem_size_reg()).
Let's respin v5 keeping minimal necessary checks and also update the
test case:
- use sub-tests.
- move raw_tp_writable_reject_nbd_invalid.c to use tracepoing from
the test module instead of nbd, to avoid nbd config dependency
(as in the attachment).
- if possible, could you please add a test that shows missing negative
offset access check to PTR_TO_BUF?
[...]
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_bad_access.c
similarity index 75%
rename from tools/testing/selftests/bpf/prog_tests/raw_tp_writable_reject_nbd_invalid.c
rename to tools/testing/selftests/bpf/prog_tests/raw_tp_writable_reject_bad_access.c
index 216b0dfac0fe..e0a1ca3c1e6a 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_bad_access.c
@@ -1,10 +1,10 @@
// SPDX-License-Identifier: GPL-2.0
#include <test_progs.h>
-#include <linux/nbd.h>
+#include "test_kmods/bpf_testmod.h"
#include "bpf_util.h"
-void test_raw_tp_writable_reject_nbd_invalid(void)
+void test_raw_tp_writable_reject_bad_access(void)
{
__u32 duration = 0;
char error[4096];
@@ -13,9 +13,9 @@ void test_raw_tp_writable_reject_nbd_invalid(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 nbd_request struct */
+ /* one byte beyond the end of the writable context struct */
BPF_LDX_MEM(BPF_B, BPF_REG_0, BPF_REG_6,
- sizeof(struct nbd_request)),
+ sizeof(struct bpf_testmod_test_writable_ctx)),
BPF_EXIT_INSN(),
};
@@ -32,7 +32,7 @@ void test_raw_tp_writable_reject_nbd_invalid(void)
"failed: %d errno %d\n", bpf_fd, errno))
return;
- tp_fd = bpf_raw_tracepoint_open("nbd_send_request", bpf_fd);
+ tp_fd = bpf_raw_tracepoint_open("bpf_testmod_test_writable_bare_tp", bpf_fd);
if (CHECK(tp_fd >= 0, "bpf_raw_tracepoint_writable open",
"erroneously succeeded\n"))
goto out_bpffd;
