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

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

[Sun Jian]

Reject negative effective offsets for PTR_TO_TP_BUFFER and PTR_TO_BUF
buffer accesses, and add raw tracepoint writable coverage for both
load-time rejection and the attach-time max_tp_access path.

---

Changes in v4:
- Correct the Fixes tag to point to 022ac0750883, where pointer offsets
  were folded into reg->var_off.
- Drop the end > U32_MAX check, which is unreachable after bounding const
  var_off with BPF_MAX_VAR_OFF while keeping instruction offsets and access
  sizes bounded.

Changes in v3:
- Check constant var_off against +/-BPF_MAX_VAR_OFF before computing
  the effective access range, matching the existing verifier pointer
  offset convention.
- Keep explicit rejection of negative instruction offsets and keep
  bounded negative constant var_off valid when the effective offset is
  non-negative.

Changes in v2:
- Split the kernel fix and selftests into separate patches.
- Add an attach-time raw tracepoint writable test that exercises
  max_tp_access against nbd_send_request's writable size.
- Adjust selftest formatting to use the 100 character line width.

Tested:
- ./test_progs -t verifier_raw_tp_writable
- ./test_progs -t raw_tp_writable_reject_nbd_invalid -v
- ./test_progs -t raw_tp_writable_test_run

v3: https://lore.kernel.org/bpf/20260708040715.116680-1-sun.jian.kdev@gmail.com/
v2: https://lore.kernel.org/bpf/20260707060804.93561-1-sun.jian.kdev@gmail.com/
v1: https://lore.kernel.org/bpf/20260703035137.109608-1-sun.jian.kdev@gmail.com/

Sun Jian (2):
  bpf: Reject negative const offsets for buffer pointers
  selftests/bpf: Cover negative raw_tp writable buffer offsets

 kernel/bpf/verifier.c                         | 40 +++++++++++--
 .../raw_tp_writable_reject_nbd_invalid.c      | 58 +++++++++++--------
 .../bpf/progs/verifier_raw_tp_writable.c      | 16 +++++
 3 files changed, 85 insertions(+), 29 deletions(-)


base-commit: 12091470c6b4c1c14b2de12dcbae2ada6cb6d20b
-- 
2.43.0

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

[Sun Jian]

The verifier rejects variable offsets for PTR_TO_TP_BUFFER and PTR_TO_BUF
accesses, but it currently accepts a constant negative offset produced by
pointer arithmetic.

For example, a raw tracepoint writable program can load ctx[0] as a
PTR_TO_TP_BUFFER, move it by -8, and then access the adjusted pointer.
The access is before the tracepoint writable buffer base and should be
rejected.

Keep rejecting negative instruction offsets explicitly. Once the register
offset is known to be constant, reject const var_off values outside the
same +/-BPF_MAX_VAR_OFF range used by other verifier pointer offset
checks before computing the effective access range.

Fixes: 022ac0750883 ("bpf: use reg->var_off instead of reg->off for pointers")
Signed-off-by: Sun Jian <sun.jian.kdev@gmail.com>
---
 kernel/bpf/verifier.c | 40 ++++++++++++++++++++++++++++++++++------
 1 file changed, 34 insertions(+), 6 deletions(-)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 21a365d436a5..6fe4fcf93e5f 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -5326,14 +5326,18 @@ static int check_max_stack_depth(struct bpf_verifier_env *env)
 static int __check_buffer_access(struct bpf_verifier_env *env,
 				 const char *buf_info,
 				 const struct bpf_reg_state *reg,
-				 argno_t argno, int off, int size)
+				 argno_t argno, int off, int size,
+				 u32 *access_end)
 {
+	s64 start, var_off;
+
 	if (off < 0) {
 		verbose(env,
 			"%s invalid %s buffer access: off=%d, size=%d\n",
 			reg_arg_name(env, argno), buf_info, off, size);
 		return -EACCES;
 	}
+
 	if (!tnum_is_const(reg->var_off)) {
 		char tn_buf[48];
 
@@ -5344,6 +5348,29 @@ static int __check_buffer_access(struct bpf_verifier_env *env,
 		return -EACCES;
 	}
 
+	var_off = (s64)reg->var_off.value;
+	if (var_off >= BPF_MAX_VAR_OFF || var_off <= -BPF_MAX_VAR_OFF) {
+		verbose(env, "%s %s buffer offset %lld is not allowed\n",
+			reg_arg_name(env, argno), buf_info, var_off);
+		return -EACCES;
+	}
+
+	start = var_off + off;
+	if (start < 0) {
+		verbose(env,
+			"%s invalid negative %s buffer offset: off=%d, var_off=%lld\n",
+			reg_arg_name(env, argno), buf_info, off, var_off);
+		return -EACCES;
+	}
+
+	if (size < 0) {
+		verbose(env, "%s invalid %s buffer access: off=%lld, size=%d\n",
+			reg_arg_name(env, argno), buf_info, start, size);
+		return -EACCES;
+	}
+
+	*access_end = (u32)start + (u32)size;
+
 	return 0;
 }
 
@@ -5351,14 +5378,14 @@ static int check_tp_buffer_access(struct bpf_verifier_env *env,
 				  const struct bpf_reg_state *reg,
 				  argno_t argno, int off, int size)
 {
+	u32 access_end;
 	int err;
 
-	err = __check_buffer_access(env, "tracepoint", reg, argno, off, size);
+	err = __check_buffer_access(env, "tracepoint", reg, argno, off, size, &access_end);
 	if (err)
 		return err;
 
-	env->prog->aux->max_tp_access = max(reg->var_off.value + off + size,
-					    env->prog->aux->max_tp_access);
+	env->prog->aux->max_tp_access = max(access_end, env->prog->aux->max_tp_access);
 
 	return 0;
 }
@@ -5370,13 +5397,14 @@ static int check_buffer_access(struct bpf_verifier_env *env,
 			       u32 *max_access)
 {
 	const char *buf_info = type_is_rdonly_mem(reg->type) ? "rdonly" : "rdwr";
+	u32 access_end;
 	int err;
 
-	err = __check_buffer_access(env, buf_info, reg, argno, off, size);
+	err = __check_buffer_access(env, buf_info, reg, argno, off, size, &access_end);
 	if (err)
 		return err;
 
-	*max_access = max(reg->var_off.value + off + size, *max_access);
+	*max_access = max(access_end, *max_access);
 
 	return 0;
 }
-- 
2.43.0

[PATCH bpf v4 2/2] selftests/bpf: Cover negative raw_tp writable buffer offsets

[Sun Jian]

Add raw tracepoint writable coverage for buffer accesses involving
negative constant pointer adjustments.

The verifier case checks that a negative effective offset is rejected.
The attach-time case adds incremental coverage beyond the existing nbd
test and the verifier rejection case: it uses a negative var_off and a
positive instruction offset whose effective offset remains non-negative.
This exercises the checked access_end accounting path and verifies that
the resulting max_tp_access is still checked against nbd_send_request's
writable size at attach time.

Signed-off-by: Sun Jian <sun.jian.kdev@gmail.com>
---
 .../raw_tp_writable_reject_nbd_invalid.c      | 58 +++++++++++--------
 .../bpf/progs/verifier_raw_tp_writable.c      | 16 +++++
 2 files changed, 51 insertions(+), 23 deletions(-)

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..efe4cad47b28 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,31 @@
 #include <linux/nbd.h>
 #include "bpf_util.h"
 
-void test_raw_tp_writable_reject_nbd_invalid(void)
+static void check_nbd_attach_reject(const char *name,
+				    const struct bpf_insn *program, size_t prog_len)
 {
-	__u32 duration = 0;
+	LIBBPF_OPTS(bpf_prog_load_opts, opts);
 	char error[4096];
-	int bpf_fd = -1, tp_fd = -1;
+	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("nbd_send_request", bpf_fd);
+	if (!ASSERT_LT(tp_fd, 0, name))
+		close(tp_fd);
+
+	close(bpf_fd);
+}
 
+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),
@@ -19,25 +38,18 @@ 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),
-	);
-
-	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;
+	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 nbd_request struct */
+		BPF_LDX_MEM(BPF_B, BPF_REG_0, BPF_REG_6,
+			    sizeof(struct nbd_request) + 8),
+		BPF_EXIT_INSN(),
+	};
 
-	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));
 }
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

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

[Shung-Hsi Yu]

On Wed, Jul 08, 2026 at 02:01:50AM -0700, Sun Jian wrote:
[...]
> Fixes: 022ac0750883 ("bpf: use reg->var_off instead of reg->off for pointers")

Agree that above is the commit that introduced the issue.

More comments below.

[...]
> @@ -5326,14 +5326,18 @@ static int check_max_stack_depth(struct bpf_verifier_env *env)
>  static int __check_buffer_access(struct bpf_verifier_env *env,
>  				 const char *buf_info,
>  				 const struct bpf_reg_state *reg,
> -				 argno_t argno, int off, int size)
> +				 argno_t argno, int off, int size,
> +				 u32 *access_end)
>  {
> +	s64 start, var_off;
> +
>  	if (off < 0) {
>  		verbose(env,
>  			"%s invalid %s buffer access: off=%d, size=%d\n",
>  			reg_arg_name(env, argno), buf_info, off, size);
>  		return -EACCES;
>  	}
> +
>  	if (!tnum_is_const(reg->var_off)) {
>  		char tn_buf[48];
>  
> @@ -5344,6 +5348,29 @@ static int __check_buffer_access(struct bpf_verifier_env *env,
>  		return -EACCES;
>  	}
>  
> +	var_off = (s64)reg->var_off.value;
> +	if (var_off >= BPF_MAX_VAR_OFF || var_off <= -BPF_MAX_VAR_OFF) {
> +		verbose(env, "%s %s buffer offset %lld is not allowed\n",
> +			reg_arg_name(env, argno), buf_info, var_off);
> +		return -EACCES;
> +	}
> +
> +	start = var_off + off;
> +	if (start < 0) {
> +		verbose(env,
> +			"%s invalid negative %s buffer offset: off=%d, var_off=%lld\n",
> +			reg_arg_name(env, argno), buf_info, off, var_off);
> +		return -EACCES;
> +	}

I was thinking of suggest to just do a single unsigned check

  var_off = reg->var_off.value;
  if (var_off >= BPF_MAX_VAR_OFF) {
    ...

But looking at the code before 022ac0750883, what you have is closer
aligned to the previous behavior, let's stick to this.

> +
> +	if (size < 0) {
> +		verbose(env, "%s invalid %s buffer access: off=%lld, size=%d\n",
> +			reg_arg_name(env, argno), buf_info, start, size);
> +		return -EACCES;
> +	}

`size` comes from bpf_size_to_bytes(bpf_size) in check_mem_access(), and
is already checked to be not negative; plus other check_* helpers does
not check for this, so I think it can be dropped.

Otherwise, LGTM:

Acked-by: Shung-Hsi Yu <shung-hsi.yu@suse.com>

> +
> +	*access_end = (u32)start + (u32)size;

If you want, this could use a quick one-line comments regarding the fact
that it won't overflow, or even verifier_bug_if().

> +
>  	return 0;
>  }
>  
[...]

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

[sun jian]

On Wed, Jul 8, 2026 at 9:13 PM Shung-Hsi Yu <shung-hsi.yu@suse.com> wrote:
>
> On Wed, Jul 08, 2026 at 02:01:50AM -0700, Sun Jian wrote:
> [...]
> > Fixes: 022ac0750883 ("bpf: use reg->var_off instead of reg->off for pointers")
>
> Agree that above is the commit that introduced the issue.
>
> More comments below.
>
> [...]
> > @@ -5326,14 +5326,18 @@ static int check_max_stack_depth(struct bpf_verifier_env *env)
> >  static int __check_buffer_access(struct bpf_verifier_env *env,
> >                                const char *buf_info,
> >                                const struct bpf_reg_state *reg,
> > -                              argno_t argno, int off, int size)
> > +                              argno_t argno, int off, int size,
> > +                              u32 *access_end)
> >  {
> > +     s64 start, var_off;
> > +
> >       if (off < 0) {
> >               verbose(env,
> >                       "%s invalid %s buffer access: off=%d, size=%d\n",
> >                       reg_arg_name(env, argno), buf_info, off, size);
> >               return -EACCES;
> >       }
> > +
> >       if (!tnum_is_const(reg->var_off)) {
> >               char tn_buf[48];
> >
> > @@ -5344,6 +5348,29 @@ static int __check_buffer_access(struct bpf_verifier_env *env,
> >               return -EACCES;
> >       }
> >
> > +     var_off = (s64)reg->var_off.value;
> > +     if (var_off >= BPF_MAX_VAR_OFF || var_off <= -BPF_MAX_VAR_OFF) {
> > +             verbose(env, "%s %s buffer offset %lld is not allowed\n",
> > +                     reg_arg_name(env, argno), buf_info, var_off);
> > +             return -EACCES;
> > +     }
> > +
> > +     start = var_off + off;
> > +     if (start < 0) {
> > +             verbose(env,
> > +                     "%s invalid negative %s buffer offset: off=%d, var_off=%lld\n",
> > +                     reg_arg_name(env, argno), buf_info, off, var_off);
> > +             return -EACCES;
> > +     }
>
> I was thinking of suggest to just do a single unsigned check
>
>   var_off = reg->var_off.value;
>   if (var_off >= BPF_MAX_VAR_OFF) {
>     ...
>
> But looking at the code before 022ac0750883, what you have is closer
> aligned to the previous behavior, let's stick to this.
>
> > +
> > +     if (size < 0) {
> > +             verbose(env, "%s invalid %s buffer access: off=%lld, size=%d\n",
> > +                     reg_arg_name(env, argno), buf_info, start, size);
> > +             return -EACCES;
> > +     }
>
> `size` comes from bpf_size_to_bytes(bpf_size) in check_mem_access(), and
> is already checked to be not negative; plus other check_* helpers does
> not check for this, so I think it can be dropped.
>
> Otherwise, LGTM:
>
> Acked-by: Shung-Hsi Yu <shung-hsi.yu@suse.com>
>
> > +
> > +     *access_end = (u32)start + (u32)size;
>
> If you want, this could use a quick one-line comments regarding the fact
> that it won't overflow, or even verifier_bug_if().
>
> > +
> >       return 0;
> >  }
> >
> [...]

Thanks for taking another look, and thanks for the Ack.

I agree that the size check is redundant given the current call path. I’ll leave
v4 as-is for now to avoid another respin unless maintainers prefer dropping it
or adding a short comment around the access_end calculation.

Best Regards
Sun Jian

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

[Shung-Hsi Yu]

On Wed, Jul 08, 2026 at 10:11:01PM +0800, sun jian wrote:
[...]
> > > @@ -5326,14 +5326,18 @@ static int check_max_stack_depth(struct bpf_verifier_env *env)
> > >  static int __check_buffer_access(struct bpf_verifier_env *env,
> > >                                const char *buf_info,
> > >                                const struct bpf_reg_state *reg,
> > > -                              argno_t argno, int off, int size)
> > > +                              argno_t argno, int off, int size,
> > > +                              u32 *access_end)
> > >  {
> > > +     s64 start, var_off;
> > > +
> > >       if (off < 0) {
> > >               verbose(env,
> > >                       "%s invalid %s buffer access: off=%d, size=%d\n",
> > >                       reg_arg_name(env, argno), buf_info, off, size);
> > >               return -EACCES;
> > >       }
> > > +
> > >       if (!tnum_is_const(reg->var_off)) {
> > >               char tn_buf[48];
> > >
> > > @@ -5344,6 +5348,29 @@ static int __check_buffer_access(struct bpf_verifier_env *env,
> > >               return -EACCES;
> > >       }
> > >
> > > +     var_off = (s64)reg->var_off.value;
> > > +     if (var_off >= BPF_MAX_VAR_OFF || var_off <= -BPF_MAX_VAR_OFF) {
> > > +             verbose(env, "%s %s buffer offset %lld is not allowed\n",
> > > +                     reg_arg_name(env, argno), buf_info, var_off);
> > > +             return -EACCES;
> > > +     }
> > > +
> > > +     start = var_off + off;
> > > +     if (start < 0) {
> > > +             verbose(env,
> > > +                     "%s invalid negative %s buffer offset: off=%d, var_off=%lld\n",
> > > +                     reg_arg_name(env, argno), buf_info, off, var_off);
> > > +             return -EACCES;
> > > +     }
> >
> > I was thinking of suggest to just do a single unsigned check
> >
> >   var_off = reg->var_off.value;
> >   if (var_off >= BPF_MAX_VAR_OFF) {
> >     ...
> >
> > But looking at the code before 022ac0750883, what you have is closer
> > aligned to the previous behavior, let's stick to this.

Actually looking again at 022ac0750883, moving the `off < 0` after
tnum_is_const() and bringing back the `off += reg->off` removed from
check_mem_access() is perhaps the more faithful restoration of the
original behavior. 

Though reg->off no longer exists, we have to use reg->var_off.value
instead. IIUC any register of pointer type should already have its
var_off bounded to +-BPF_MAX_VAR_OFF by adjust_ptr_min_max_vals() in
theory, and thus shouldn't overflow `int off`.

See the diff below.

[...]

> I agree that the size check is redundant given the current call path. I’ll leave
> v4 as-is for now to avoid another respin unless maintainers prefer dropping it
> or adding a short comment around the access_end calculation.

Agree and make sense. Let's see what @Eduard thinks.

---

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index d46f7db20d8f..e116b33ad83e 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -5359,12 +5359,6 @@ static int __check_buffer_access(struct bpf_verifier_env *env,
 				 const struct bpf_reg_state *reg,
 				 argno_t argno, int off, int size)
 {
-	if (off < 0) {
-		verbose(env,
-			"%s invalid %s buffer access: off=%d, size=%d\n",
-			reg_arg_name(env, argno), buf_info, off, size);
-		return -EACCES;
-	}
 	if (!tnum_is_const(reg->var_off)) {
 		char tn_buf[48];
 
@@ -5375,6 +5369,14 @@ static int __check_buffer_access(struct bpf_verifier_env *env,
 		return -EACCES;
 	}
 
+	off += reg->var_off.value;
+	if (off < 0) {
+		verbose(env,
+			"%s invalid %s buffer access: off=%d, size=%d\n",
+			reg_arg_name(env, argno), buf_info, off, size);
+		return -EACCES;
+	}
+
 	return 0;
 }
 

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

[sun jian]

On Thu, Jul 9, 2026 at 2:47 PM Shung-Hsi Yu <shung-hsi.yu@suse.com> wrote:
>
> On Wed, Jul 08, 2026 at 10:11:01PM +0800, sun jian wrote:
> [...]
> > > > @@ -5326,14 +5326,18 @@ static int check_max_stack_depth(struct bpf_verifier_env *env)
> > > >  static int __check_buffer_access(struct bpf_verifier_env *env,
> > > >                                const char *buf_info,
> > > >                                const struct bpf_reg_state *reg,
> > > > -                              argno_t argno, int off, int size)
> > > > +                              argno_t argno, int off, int size,
> > > > +                              u32 *access_end)
> > > >  {
> > > > +     s64 start, var_off;
> > > > +
> > > >       if (off < 0) {
> > > >               verbose(env,
> > > >                       "%s invalid %s buffer access: off=%d, size=%d\n",
> > > >                       reg_arg_name(env, argno), buf_info, off, size);
> > > >               return -EACCES;
> > > >       }
> > > > +
> > > >       if (!tnum_is_const(reg->var_off)) {
> > > >               char tn_buf[48];
> > > >
> > > > @@ -5344,6 +5348,29 @@ static int __check_buffer_access(struct bpf_verifier_env *env,
> > > >               return -EACCES;
> > > >       }
> > > >
> > > > +     var_off = (s64)reg->var_off.value;
> > > > +     if (var_off >= BPF_MAX_VAR_OFF || var_off <= -BPF_MAX_VAR_OFF) {
> > > > +             verbose(env, "%s %s buffer offset %lld is not allowed\n",
> > > > +                     reg_arg_name(env, argno), buf_info, var_off);
> > > > +             return -EACCES;
> > > > +     }
> > > > +
> > > > +     start = var_off + off;
> > > > +     if (start < 0) {
> > > > +             verbose(env,
> > > > +                     "%s invalid negative %s buffer offset: off=%d, var_off=%lld\n",
> > > > +                     reg_arg_name(env, argno), buf_info, off, var_off);
> > > > +             return -EACCES;
> > > > +     }
> > >
> > > I was thinking of suggest to just do a single unsigned check
> > >
> > >   var_off = reg->var_off.value;
> > >   if (var_off >= BPF_MAX_VAR_OFF) {
> > >     ...
> > >
> > > But looking at the code before 022ac0750883, what you have is closer
> > > aligned to the previous behavior, let's stick to this.
>
> Actually looking again at 022ac0750883, moving the `off < 0` after
> tnum_is_const() and bringing back the `off += reg->off` removed from
> check_mem_access() is perhaps the more faithful restoration of the
> original behavior.
>
> Though reg->off no longer exists, we have to use reg->var_off.value
> instead. IIUC any register of pointer type should already have its
> var_off bounded to +-BPF_MAX_VAR_OFF by adjust_ptr_min_max_vals() in
> theory, and thus shouldn't overflow `int off`.
>
Thanks for the additional analysis.

I checked the related verifier code paths and the following provenance items:

1. The source and initial state of PTR_TO_TP_BUFFER on the raw_tp writable
path.
2. The shared PTR_TO_TP_BUFFER/PTR_TO_BUF buffer access checker path.
3. The pointer arithmetic path through adjust_ptr_min_max_vals() and the
BPF_MAX_VAR_OFF bounds.
4. The source and range guarantees of the access size.
5. The source and range of the instruction offset passed to buffer access
checks.
6.The accounting path from effective offset calculation to access_end and
max_tp_access/max_access.

Based on these checks, folding the constant var_off back into the access
offset is consistent with the current pointer offset tracking model and is
also closer to the pre-022ac0750883 behavior.

> See the diff below.
>
> [...]
>
> > I agree that the size check is redundant given the current call path. I’ll leave
> > v4 as-is for now to avoid another respin unless maintainers prefer dropping it
> > or adding a short comment around the access_end calculation.
>
> Agree and make sense. Let's see what @Eduard thinks.
>
> ---
>
One remaining question is the intended semantics after restoring the
offset-folding behavior.

For example:

    r6 += 16;
    r0 = *(u64 *)(r6 - 8);

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index d46f7db20d8f..e116b33ad83e 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -5359,12 +5359,6 @@ static int __check_buffer_access(struct bpf_verifier_env *env,
>                                  const struct bpf_reg_state *reg,
>                                  argno_t argno, int off, int size)
>  {
> -       if (off < 0) {
> -               verbose(env,
> -                       "%s invalid %s buffer access: off=%d, size=%d\n",
> -                       reg_arg_name(env, argno), buf_info, off, size);
> -               return -EACCES;
> -       }

The current v4 approach rejects this because the instruction offset is negative.

>         if (!tnum_is_const(reg->var_off)) {
>                 char tn_buf[48];
>
> @@ -5375,6 +5369,14 @@ static int __check_buffer_access(struct bpf_verifier_env *env,
>                 return -EACCES;
>         }
>
> +       off += reg->var_off.value;
> +       if (off < 0) {
> +               verbose(env,
> +                       "%s invalid %s buffer access: off=%d, size=%d\n",
> +                       reg_arg_name(env, argno), buf_info, off, size);
> +               return -EACCES;
> +       }
> +
>         return 0;
>  }
>
The folded-offset approach would accept it because the final effective
offset is non-negative.

So this would change the accepted program set if we choose to restore the
previous offset-folding behavior. It would be good to clarify whether the
effective-offset behavior or the current instruction-offset restriction is
the desired convention.

I'll wait for the maintainers' preference.

Thanks,
Sun Jian

Re: [PATCH bpf v4 2/2] selftests/bpf: Cover negative raw_tp writable buffer offsets

[Eduard Zingerman]

On Wed, 2026-07-08 at 02:01 -0700, Sun Jian wrote:
> Add raw tracepoint writable coverage for buffer accesses involving
> negative constant pointer adjustments.
> 
> The verifier case checks that a negative effective offset is rejected.
> The attach-time case adds incremental coverage beyond the existing nbd
> test and the verifier rejection case: it uses a negative var_off and a
> positive instruction offset whose effective offset remains non-negative.
> This exercises the checked access_end accounting path and verifies that
> the resulting max_tp_access is still checked against nbd_send_request's
> writable size at attach time.
> 
> Signed-off-by: Sun Jian <sun.jian.kdev@gmail.com>
> ---
>  .../raw_tp_writable_reject_nbd_invalid.c      | 58 +++++++++++--------
>  .../bpf/progs/verifier_raw_tp_writable.c      | 16 +++++
>  2 files changed, 51 insertions(+), 23 deletions(-)
> 
> 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..efe4cad47b28 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,31 @@
>  #include <linux/nbd.h>
>  #include "bpf_util.h"
>  
> -void test_raw_tp_writable_reject_nbd_invalid(void)
> +static void check_nbd_attach_reject(const char *name,
> +				    const struct bpf_insn *program, size_t prog_len)
>  {
> -	__u32 duration = 0;
> +	LIBBPF_OPTS(bpf_prog_load_opts, opts);
>  	char error[4096];
> -	int bpf_fd = -1, tp_fd = -1;
> +	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("nbd_send_request", bpf_fd);
> +	if (!ASSERT_LT(tp_fd, 0, name))
> +		close(tp_fd);
> +
> +	close(bpf_fd);
> +}
>  
> +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),
> @@ -19,25 +38,18 @@ 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),
> -	);
> -
> -	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;
> +	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 nbd_request struct */
> +		BPF_LDX_MEM(BPF_B, BPF_REG_0, BPF_REG_6,
> +			    sizeof(struct nbd_request) + 8),
> +		BPF_EXIT_INSN(),
> +	};
>  
> -	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));
>  }

Let's use subtests here:

static void check_nbd_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("nbd_send_request", bpf_fd);
	ASSERT_ERR_FD(tp_fd, "bpf_raw_tracepoint_open");

	close(tp_fd);
	close(bpf_fd);
}

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 */
		BPF_LDX_MEM(BPF_B, BPF_REG_0, BPF_REG_6,
			    sizeof(struct nbd_request)),
		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 nbd_request struct */
		BPF_LDX_MEM(BPF_B, BPF_REG_0, BPF_REG_6,
			    sizeof(struct nbd_request) + 8),
		BPF_EXIT_INSN(),
	};

	if (test__start_subtest("nbd_invalid"))
		check_nbd_attach_reject(program, ARRAY_SIZE(program));
	if (test__start_subtest("nbd_invalid_negative_var_off"))
		check_nbd_attach_reject(negative_var_off_program,
					ARRAY_SIZE(negative_var_off_program));
}


[...]

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

[Eduard Zingerman]

On Thu, 2026-07-09 at 14:47 +0800, Shung-Hsi Yu wrote:
> On Wed, Jul 08, 2026 at 10:11:01PM +0800, sun jian wrote:
> [...]
> > > > @@ -5326,14 +5326,18 @@ static int check_max_stack_depth(struct bpf_verifier_env *env)
> > > >  static int __check_buffer_access(struct bpf_verifier_env *env,
> > > >                                const char *buf_info,
> > > >                                const struct bpf_reg_state *reg,
> > > > -                              argno_t argno, int off, int size)
> > > > +                              argno_t argno, int off, int size,
> > > > +                              u32 *access_end)
> > > >  {
> > > > +     s64 start, var_off;
> > > > +
> > > >       if (off < 0) {
> > > >               verbose(env,
> > > >                       "%s invalid %s buffer access: off=%d, size=%d\n",
> > > >                       reg_arg_name(env, argno), buf_info, off, size);
> > > >               return -EACCES;
> > > >       }
> > > > +
> > > >       if (!tnum_is_const(reg->var_off)) {
> > > >               char tn_buf[48];
> > > > 
> > > > @@ -5344,6 +5348,29 @@ static int __check_buffer_access(struct bpf_verifier_env *env,
> > > >               return -EACCES;
> > > >       }
> > > > 
> > > > +     var_off = (s64)reg->var_off.value;
> > > > +     if (var_off >= BPF_MAX_VAR_OFF || var_off <= -BPF_MAX_VAR_OFF) {
> > > > +             verbose(env, "%s %s buffer offset %lld is not allowed\n",
> > > > +                     reg_arg_name(env, argno), buf_info, var_off);
> > > > +             return -EACCES;
> > > > +     }
> > > > +
> > > > +     start = var_off + off;
> > > > +     if (start < 0) {
> > > > +             verbose(env,
> > > > +                     "%s invalid negative %s buffer offset: off=%d, var_off=%lld\n",
> > > > +                     reg_arg_name(env, argno), buf_info, off, var_off);
> > > > +             return -EACCES;
> > > > +     }
> > > 
> > > I was thinking of suggest to just do a single unsigned check
> > > 
> > >   var_off = reg->var_off.value;
> > >   if (var_off >= BPF_MAX_VAR_OFF) {
> > >     ...
> > > 
> > > But looking at the code before 022ac0750883, what you have is closer
> > > aligned to the previous behavior, let's stick to this.
> 
> Actually looking again at 022ac0750883, moving the `off < 0` after
> tnum_is_const() and bringing back the `off += reg->off` removed from
> check_mem_access() is perhaps the more faithful restoration of the
> original behavior. 
> 
> Though reg->off no longer exists, we have to use reg->var_off.value
> instead. IIUC any register of pointer type should already have its
> var_off bounded to +-BPF_MAX_VAR_OFF by adjust_ptr_min_max_vals() in
> theory, and thus shouldn't overflow `int off`.
> 
> See the diff below.
> 
> [...]
> 
> > I agree that the size check is redundant given the current call path. I’ll leave
> > v4 as-is for now to avoid another respin unless maintainers prefer dropping it
> > or adding a short comment around the access_end calculation.
> 
> Agree and make sense. Let's see what @Eduard thinks.

I don't understand what this patch is attempting to fix.
If you run the selftests from patch #2 against current bpf-next both
would be rejected. If you extend these test cases to exercise a truly
negative offset, that would be rejected as well.

And this does not rely on UB.
Consider the current code:

	env->prog->aux->max_tp_access = max(reg->var_off.value + off + size,
					    env->prog->aux->max_tp_access);


The types of the expressions involved:

  reg->var_off.value + off + size
  u64                  int   int

The promotion/conversion rules:

  u64 + (u64)(s64)int -> u64

In other words, 'off' and 'size' would be sign extended to s64 and
then treated as u64. Hence any negative offset would be represented
as a large unsigned value in max_tp_access.

Now, the real head scratcher is whether u64 -> u32 truncation on the
assignment can bite here (mask higher bits of a big positive or
negative value). Given the bounds on the values involved:
- reg->var_off.value is -2**29..2**29
- off is -2**16+1..2**16-1
- size is 1, 2, 4 or 8

It cannot.

If one wants to be truly paranoid, one can add a truncation check
before the assignment, just to avoid the mental gymnastics.
But that won't be a fix for any observable behaviour.

[...]

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

[sun jian]

On Fri, Jul 10, 2026 at 2:21 AM Eduard Zingerman <eddyz87@gmail.com> wrote:
>
> On Thu, 2026-07-09 at 14:47 +0800, Shung-Hsi Yu wrote:
> > On Wed, Jul 08, 2026 at 10:11:01PM +0800, sun jian wrote:
> > [...]
> > > > > @@ -5326,14 +5326,18 @@ static int check_max_stack_depth(struct bpf_verifier_env *env)
> > > > >  static int __check_buffer_access(struct bpf_verifier_env *env,
> > > > >                                const char *buf_info,
> > > > >                                const struct bpf_reg_state *reg,
> > > > > -                              argno_t argno, int off, int size)
> > > > > +                              argno_t argno, int off, int size,
> > > > > +                              u32 *access_end)
> > > > >  {
> > > > > +     s64 start, var_off;
> > > > > +
> > > > >       if (off < 0) {
> > > > >               verbose(env,
> > > > >                       "%s invalid %s buffer access: off=%d, size=%d\n",
> > > > >                       reg_arg_name(env, argno), buf_info, off, size);
> > > > >               return -EACCES;
> > > > >       }
> > > > > +
> > > > >       if (!tnum_is_const(reg->var_off)) {
> > > > >               char tn_buf[48];
> > > > >
> > > > > @@ -5344,6 +5348,29 @@ static int __check_buffer_access(struct bpf_verifier_env *env,
> > > > >               return -EACCES;
> > > > >       }
> > > > >
> > > > > +     var_off = (s64)reg->var_off.value;
> > > > > +     if (var_off >= BPF_MAX_VAR_OFF || var_off <= -BPF_MAX_VAR_OFF) {
> > > > > +             verbose(env, "%s %s buffer offset %lld is not allowed\n",
> > > > > +                     reg_arg_name(env, argno), buf_info, var_off);
> > > > > +             return -EACCES;
> > > > > +     }
> > > > > +
> > > > > +     start = var_off + off;
> > > > > +     if (start < 0) {
> > > > > +             verbose(env,
> > > > > +                     "%s invalid negative %s buffer offset: off=%d, var_off=%lld\n",
> > > > > +                     reg_arg_name(env, argno), buf_info, off, var_off);
> > > > > +             return -EACCES;
> > > > > +     }
> > > >
> > > > I was thinking of suggest to just do a single unsigned check
> > > >
> > > >   var_off = reg->var_off.value;
> > > >   if (var_off >= BPF_MAX_VAR_OFF) {
> > > >     ...
> > > >
> > > > But looking at the code before 022ac0750883, what you have is closer
> > > > aligned to the previous behavior, let's stick to this.
> >
> > Actually looking again at 022ac0750883, moving the `off < 0` after
> > tnum_is_const() and bringing back the `off += reg->off` removed from
> > check_mem_access() is perhaps the more faithful restoration of the
> > original behavior.
> >
> > Though reg->off no longer exists, we have to use reg->var_off.value
> > instead. IIUC any register of pointer type should already have its
> > var_off bounded to +-BPF_MAX_VAR_OFF by adjust_ptr_min_max_vals() in
> > theory, and thus shouldn't overflow `int off`.
> >
> > See the diff below.
> >
> > [...]
> >
> > > I agree that the size check is redundant given the current call path. I’ll leave
> > > v4 as-is for now to avoid another respin unless maintainers prefer dropping it
> > > or adding a short comment around the access_end calculation.
> >
> > Agree and make sense. Let's see what @Eduard thinks.
>
> I don't understand what this patch is attempting to fix.
> If you run the selftests from patch #2 against current bpf-next both
> would be rejected. If you extend these test cases to exercise a truly
> negative offset, that would be rejected as well.
>

Thanks for pushing on this. I rechecked the issue more carefully.

This series targets the bpf tree, with base 12091470c6b4. On that base,
with only the selftest change applied, the negative-offset verifier case
is not rejected at load time. The test fails with an unexpected load
success:

#664/2 verifier_raw_tp_writable/raw_tracepoint_writable: reject
negative const offset:FAIL
run_subtest:FAIL:unexpected_load_success unexpected success: 0

It is possible that current bpf-next rejects this earlier through another
path, but on the target bpf base it does not.

> And this does not rely on UB.
> Consider the current code:
>
>         env->prog->aux->max_tp_access = max(reg->var_off.value + off + size,
>                                             env->prog->aux->max_tp_access);
>
>
> The types of the expressions involved:
>
>   reg->var_off.value + off + size
>   u64                  int   int
>
> The promotion/conversion rules:
>
>   u64 + (u64)(s64)int -> u64
>
> In other words, 'off' and 'size' would be sign extended to s64 and
> then treated as u64. Hence any negative offset would be represented
> as a large unsigned value in max_tp_access.
>

Agreed, this is not a u64-to-u32 truncation issue. For the problematic
case the access range is [-8, 0), so the end offset is 0. Since
max_tp_access only tracks the upper bound, the attach-time writable_size
check cannot catch the negative start offset. The missing piece is the
lower-bound check.

> Now, the real head scratcher is whether u64 -> u32 truncation on the
> assignment can bite here (mask higher bits of a big positive or
> negative value). Given the bounds on the values involved:
> - reg->var_off.value is -2**29..2**29
> - off is -2**16+1..2**16-1
> - size is 1, 2, 4 or 8
>
> It cannot.
>
> If one wants to be truly paranoid, one can add a truncation check
> before the assignment, just to avoid the mental gymnastics.
> But that won't be a fix for any observable behaviour.
>

There is an observable effect on the target bpf base. I tested it with a
temporary raw_tp writable test_run reproducer, not part of the series. It
attaches a raw_tp writable program to bpf_test_finish and triggers it
through BPF_PROG_TEST_RUN. The program does:

r6 = *(u64 *)(r1 + 0)
r6 += -8
*(u64 *)(r6 + 0) = 0

With KASAN enabled and the BPF JIT disabled, on the bpf base without the
verifier fix, the program loads, attaches, executes, and KASAN reports:

BUG: KASAN: stack-out-of-bounds in ___bpf_prog_run
Write of size 8

with the relevant call trace:

___bpf_prog_run
__bpf_prog_run32
bpf_trace_run1
bpf_test_finish
bpf_prog_test_run_skb
bpf_prog_test_run
__sys_bpf

The report points to the bpf_test_finish stack frame, which matches the
raw_tp writable program being attached to bpf_test_finish.

With this series applied, the same negative-offset access is rejected at
load time:

R6 invalid negative tracepoint buffer offset: off=0, var_off=-8

Rerunning the same reproducer against the fixed kernel, the negative
program is rejected by bpf_prog_load() and the KASAN log stays clean:

NEGATIVE_RUNTIME_REPRO: negative_bpf_load rejected at load time: fd=-13 errno=13

So the observable behavior is the missing lower-bound check: on the
target bpf base, the verifier can accept and execute an access before the
writable tracepoint buffer base. The patch prevents that by rejecting
negative effective buffer offsets at verification time.

Thanks,
Sun Jian

> [...]

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

[Eduard Zingerman]

On Fri, 2026-07-10 at 13:52 +0800, sun jian wrote:

[...]

> There is an observable effect on the target bpf base. I tested it with a
> temporary raw_tp writable test_run reproducer, not part of the series. It
> attaches a raw_tp writable program to bpf_test_finish and triggers it
> through BPF_PROG_TEST_RUN. The program does:
> 
> r6 = *(u64 *)(r1 + 0)
> r6 += -8
> *(u64 *)(r6 + 0) = 0
>
> With KASAN enabled and the BPF JIT disabled, on the bpf base without the
> verifier fix, the program loads, attaches, executes, and KASAN reports:

Could you please identify why the behaviour differs between bpf and bpf-next?

[...]

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

[sun jian]

On Fri, Jul 10, 2026 at 2:23 PM Eduard Zingerman <eddyz87@gmail.com> wrote:
>
> On Fri, 2026-07-10 at 13:52 +0800, sun jian wrote:
>
> [...]
>
> > There is an observable effect on the target bpf base. I tested it with a
> > temporary raw_tp writable test_run reproducer, not part of the series. It
> > attaches a raw_tp writable program to bpf_test_finish and triggers it
> > through BPF_PROG_TEST_RUN. The program does:
> >
> > r6 = *(u64 *)(r1 + 0)
> > r6 += -8
> > *(u64 *)(r6 + 0) = 0
> >
> > With KASAN enabled and the BPF JIT disabled, on the bpf base without the
> > verifier fix, the program loads, attaches, executes, and KASAN reports:
>
> Could you please identify why the behaviour differs between bpf and bpf-next?

I checked the current bpf-next, but I could not reproduce the difference.

Both the public cgit page and my local bpf-next/master and bpf-next/for-next
point to:
a4553044d1af ("Merge branch 'bpf-bound-rdonly-rdwr_buf_size-kfunc-return-size'")

I tested that tree with only the selftest patch applied. The staged diff
only contains:

tools/testing/selftests/bpf/prog_tests/raw_tp_writable_reject_nbd_invalid.c
tools/testing/selftests/bpf/progs/verifier_raw_tp_writable.c

The negative const-offset case is still accepted at load time:

#637/2 verifier_raw_tp_writable/raw_tracepoint_writable: reject
negative const offset:FAIL
run_subtest:FAIL:unexpected_load_success unexpected success: 0

and the verifier log only shows:

processed 4 insns (limit 1000000) max_states_per_insn 0 total_states 0
peak_states 0 mark_read 0

The current __check_buffer_access() in that tree still only rejects a
negative instruction offset and a non-constant var_off. It does not check
the folded effective offset, so this case still has off=0 and var_off=-8 and
is accepted.

So, at least on a4553044d1af, I do not see bpf-next rejecting this case. If
you have a bpf-next commit or verifier log where this case is rejected, I'd
be glad to take a look and reconcile; I may well be missing something in my
setup.

Thanks,
Sun Jian
>
> [...]

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

[Eduard Zingerman]

On Fri, 2026-07-10 at 15:25 +0800, sun jian wrote:
> On Fri, Jul 10, 2026 at 2:23 PM Eduard Zingerman <eddyz87@gmail.com> wrote:
> > 
> > On Fri, 2026-07-10 at 13:52 +0800, sun jian wrote:
> > 
> > [...]
> > 
> > > There is an observable effect on the target bpf base. I tested it with a
> > > temporary raw_tp writable test_run reproducer, not part of the series. It
> > > attaches a raw_tp writable program to bpf_test_finish and triggers it
> > > through BPF_PROG_TEST_RUN. The program does:
> > > 
> > > r6 = *(u64 *)(r1 + 0)
> > > r6 += -8
> > > *(u64 *)(r6 + 0) = 0
> > > 
> > > With KASAN enabled and the BPF JIT disabled, on the bpf base without the
> > > verifier fix, the program loads, attaches, executes, and KASAN reports:
> > 
> > Could you please identify why the behaviour differs between bpf and bpf-next?
> 
> I checked the current bpf-next, but I could not reproduce the difference.
> 
> Both the public cgit page and my local bpf-next/master and bpf-next/for-next
> point to:
> a4553044d1af ("Merge branch 'bpf-bound-rdonly-rdwr_buf_size-kfunc-return-size'")
> 
> I tested that tree with only the selftest patch applied. The staged diff
> only contains:
> 
> tools/testing/selftests/bpf/prog_tests/raw_tp_writable_reject_nbd_invalid.c
> tools/testing/selftests/bpf/progs/verifier_raw_tp_writable.c
> 
> The negative const-offset case is still accepted at load time:
> 
> #637/2 verifier_raw_tp_writable/raw_tracepoint_writable: reject
> negative const offset:FAIL
> run_subtest:FAIL:unexpected_load_success unexpected success: 0
> 
> and the verifier log only shows:
> 
> processed 4 insns (limit 1000000) max_states_per_insn 0 total_states 0
> peak_states 0 mark_read 0

Here is the branch:
https://github.com/eddyz87/bpf/tree/tp-buffer-negative-offset
It is the current bpf-next HEAD [1] plus patches #1,2 and a revert of patch #1.

I have KASAN enabled, the test is passing:

#317     raw_tp_writable_reject_nbd_invalid:OK

[1] c3d5ef291a2a ("selftests/bpf: Add test for scalar id on sign-extending stack fill")

> The current __check_buffer_access() in that tree still only rejects a
> negative instruction offset and a non-constant var_off. It does not check
> the folded effective offset, so this case still has off=0 and var_off=-8 and
> is accepted.
> 
> So, at least on a4553044d1af, I do not see bpf-next rejecting this case. If
> you have a bpf-next commit or verifier log where this case is rejected, I'd
> be glad to take a look and reconcile; I may well be missing something in my
> setup.
> 
> Thanks,
> Sun Jian
> > 
> > [...]

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

[Shung-Hsi Yu]

On Fri, Jul 10, 2026 at 01:52:26PM +0800, sun jian wrote:
> On Fri, Jul 10, 2026 at 2:21 AM Eduard Zingerman <eddyz87@gmail.com> wrote:
> > On Thu, 2026-07-09 at 14:47 +0800, Shung-Hsi Yu wrote:
> > > On Wed, Jul 08, 2026 at 10:11:01PM +0800, sun jian wrote:
[...]
> > > Actually looking again at 022ac0750883, moving the `off < 0` after
> > > tnum_is_const() and bringing back the `off += reg->off` removed from
> > > check_mem_access() is perhaps the more faithful restoration of the
> > > original behavior.
> > >
> > > Though reg->off no longer exists, we have to use reg->var_off.value
> > > instead. IIUC any register of pointer type should already have its
> > > var_off bounded to +-BPF_MAX_VAR_OFF by adjust_ptr_min_max_vals() in
> > > theory, and thus shouldn't overflow `int off`.
> > >
> > > See the diff below.
[...]
> >
> > I don't understand what this patch is attempting to fix.
> > If you run the selftests from patch #2 against current bpf-next both
> > would be rejected. If you extend these test cases to exercise a truly
> > negative offset, that would be rejected as well.

I tried to exercise a truly negative offset, and it was indeed rejected
on bpf-next. I had thought it would pass.

But more below.

> Thanks for pushing on this. I rechecked the issue more carefully.
> 
> This series targets the bpf tree, with base 12091470c6b4. On that base,
> with only the selftest change applied, the negative-offset verifier case
> is not rejected at load time. The test fails with an unexpected load
> success:
> 
> #664/2 verifier_raw_tp_writable/raw_tracepoint_writable: reject
> negative const offset:FAIL
> run_subtest:FAIL:unexpected_load_success unexpected success: 0
> 
> It is possible that current bpf-next rejects this earlier through another
> path, but on the target bpf base it does not.

So I think I now have a better picture of what's going on. Before
022ac0750883, negative const offset is rejected at *load* time because
the const offset is accumulated into `off` before the negative check:

    // check_mem_access()
    off += reg->off
	// __check_buffer_access()
	if (off < 0) return return -EACCES;
	if (!tnum_is_const(reg->var_off) || reg->var_off.value) return -EACCES;

After 022ac0750883 the const offset accumulation does not happen, and
thus now negative const offset is no longer rejected at load time.

	// __check_buffer_access()
	if (off < 0) return return -EACCES;
	if (!tnum_is_const(reg->var_off)) return -EACCES;

> > And this does not rely on UB.
> > Consider the current code:
> >
> >         env->prog->aux->max_tp_access = max(reg->var_off.value + off + size,
> >                                             env->prog->aux->max_tp_access);
> >
> >
> > The types of the expressions involved:
> >
> >   reg->var_off.value + off + size
> >   u64                  int   int
> >
> > The promotion/conversion rules:
> >
> >   u64 + (u64)(s64)int -> u64
> >
> > In other words, 'off' and 'size' would be sign extended to s64 and
> > then treated as u64. Hence any negative offset would be represented
> > as a large unsigned value in max_tp_access.

I entirely missed that, yes, the attached-time check does prevented the
negative offset going through. So there isn't a bug, but the behavior
has changed, and it seems better to restore to the one where we
straightout reject negative offset during load time.

That can be done by reordering `off < 0` check after tnum_is_const(),
similar to how check_ptr_to_btf_access() was update in commit
022ac0750883.

[...]

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

[Shung-Hsi Yu]

Forgot to include the verifier log when running
verifier_raw_tp_writable/raw_tracepoint_writable as evidence.

On Fri, Jul 10, 2026 at 04:00:27PM +0800, Shung-Hsi Yu wrote:
> So I think I now have a better picture of what's going on. Before
> 022ac0750883, negative const offset is rejected at *load* time because
> the const offset is accumulated into `off` before the negative check:
> 
>     // check_mem_access()
>     off += reg->off
> 	// __check_buffer_access()
> 	if (off < 0) return return -EACCES;
> 	if (!tnum_is_const(reg->var_off) || reg->var_off.value) return -EACCES;

On 136114e0abf0 ("Merge tag 'mm-nonmm-stable-2026-02-12-10-48'...") I
get:

  libbpf: prog 'tracepoint_writable_reject_negative_const_offset': BPF program load failed: -EACCES
  ...
  0: R1=ctx() R10=fp0
  ; asm volatile ("					\ @ verifier_raw_tp_writable.c:56
  0: (79) r6 = *(u64 *)(r1 +0)          ; R1=ctx() R6=tp_buffer()
  1: (07) r6 += -8                      ; R6=tp_buffer(off=-8)
  2: (79) r0 = *(u64 *)(r6 +0)
  R6 invalid tracepoint buffer access: off=-8, size=8
  processed 3 insns (limit 1000000) max_states_per_insn 0 total_states 0 peak_states 0 mark_read 0

> After 022ac0750883 the const offset accumulation does not happen, and
> thus now negative const offset is no longer rejected at load time.
> 
> 	// __check_buffer_access()
> 	if (off < 0) return return -EACCES;
> 	if (!tnum_is_const(reg->var_off)) return -EACCES;

On a4553044d1af ("Merge branch
'bpf-bound-rdonly-rdwr_buf_size-kfunc-return-size'") I get

  0: R1=ctx() R10=fp0
  ; asm volatile ("					\ @ verifier_raw_tp_writable.c:56
  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
  
> [...]

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

[sun jian]

On Fri, Jul 10, 2026 at 4:00 PM Shung-Hsi Yu <shung-hsi.yu@suse.com> wrote:
>
> On Fri, Jul 10, 2026 at 01:52:26PM +0800, sun jian wrote:
> > On Fri, Jul 10, 2026 at 2:21 AM Eduard Zingerman <eddyz87@gmail.com> wrote:
> > > On Thu, 2026-07-09 at 14:47 +0800, Shung-Hsi Yu wrote:
> > > > On Wed, Jul 08, 2026 at 10:11:01PM +0800, sun jian wrote:
> [...]
> > > > Actually looking again at 022ac0750883, moving the `off < 0` after
> > > > tnum_is_const() and bringing back the `off += reg->off` removed from
> > > > check_mem_access() is perhaps the more faithful restoration of the
> > > > original behavior.
> > > >
> > > > Though reg->off no longer exists, we have to use reg->var_off.value
> > > > instead. IIUC any register of pointer type should already have its
> > > > var_off bounded to +-BPF_MAX_VAR_OFF by adjust_ptr_min_max_vals() in
> > > > theory, and thus shouldn't overflow `int off`.
> > > >
> > > > See the diff below.
> [...]
> > >
> > > I don't understand what this patch is attempting to fix.
> > > If you run the selftests from patch #2 against current bpf-next both
> > > would be rejected. If you extend these test cases to exercise a truly
> > > negative offset, that would be rejected as well.
>
> I tried to exercise a truly negative offset, and it was indeed rejected
> on bpf-next. I had thought it would pass.
>
> But more below.
>
> > Thanks for pushing on this. I rechecked the issue more carefully.
> >
> > This series targets the bpf tree, with base 12091470c6b4. On that base,
> > with only the selftest change applied, the negative-offset verifier case
> > is not rejected at load time. The test fails with an unexpected load
> > success:
> >
> > #664/2 verifier_raw_tp_writable/raw_tracepoint_writable: reject
> > negative const offset:FAIL
> > run_subtest:FAIL:unexpected_load_success unexpected success: 0
> >
> > It is possible that current bpf-next rejects this earlier through another
> > path, but on the target bpf base it does not.
>
> So I think I now have a better picture of what's going on. Before
> 022ac0750883, negative const offset is rejected at *load* time because
> the const offset is accumulated into `off` before the negative check:
>
>     // check_mem_access()
>     off += reg->off
>         // __check_buffer_access()
>         if (off < 0) return return -EACCES;
>         if (!tnum_is_const(reg->var_off) || reg->var_off.value) return -EACCES;
>
> After 022ac0750883 the const offset accumulation does not happen, and
> thus now negative const offset is no longer rejected at load time.
>
>         // __check_buffer_access()
>         if (off < 0) return return -EACCES;
>         if (!tnum_is_const(reg->var_off)) return -EACCES;
>
> > > And this does not rely on UB.
> > > Consider the current code:
> > >
> > >         env->prog->aux->max_tp_access = max(reg->var_off.value + off + size,
> > >                                             env->prog->aux->max_tp_access);
> > >
> > >
> > > The types of the expressions involved:
> > >
> > >   reg->var_off.value + off + size
> > >   u64                  int   int
> > >
> > > The promotion/conversion rules:
> > >
> > >   u64 + (u64)(s64)int -> u64
> > >
> > > In other words, 'off' and 'size' would be sign extended to s64 and
> > > then treated as u64. Hence any negative offset would be represented
> > > as a large unsigned value in max_tp_access.
>

Thanks for checking this and for the verifier logs.
The before/after logs match what I see as well. Before 022ac0750883, the
constant pointer offset was folded into off, so this case was rejected at
load time with:
    R6 invalid tracepoint buffer access: off=-8, size=8

After 022ac0750883, the same constant offset remains in reg->var_off.value,
and the program is accepted at load time with:
    R6=tp_buffer(imm=-8)
    processed 4 insns

So I agree that the behavior change comes from 022ac0750883, and that the
right direction is to restore the load-time rejection of negative effective
buffer offsets.

> I entirely missed that, yes, the attached-time check does prevented the
> negative offset going through. So there isn't a bug, but the behavior
> has changed, and it seems better to restore to the one where we
> straightout reject negative offset during load time.
>

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.

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.

In any case, I agree that v5 should restore the previous load-time rejection
semantics. For v5, I'm inclined to follow the direction you suggested: fold the
constant reg->var_off.value into off before the negative check, similar to the
022ac0750883 handling in check_ptr_to_btf_access(), rather than keeping the v4
explicit access_end form. I'll hold off on respinning until you and Eduard are
settled on the approach.

> That can be done by reordering `off < 0` check after tnum_is_const(),
> similar to how check_ptr_to_btf_access() was update in commit
> 022ac0750883.
>
> [...]