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

[PATCH bpf v2 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.

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

Changes in v2:
- Split the kernel fix and selftests into separate patches.
- Add attach-time raw tracepoint writable coverage for a negative
  var_off whose effective offset stays 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.
- 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

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 v2 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.

Compute the signed effective buffer offset before updating max access
accounting. Reject negative effective offsets and use the checked access
end for max_tp_access and other buffer max access accounting.

Fixes: 9df1c28bb752 ("bpf: add writable context for raw tracepoints")
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..b4ed6d519630 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 (check_add_overflow(var_off, (s64)off, &start)) {
+		verbose(env,
+			"%s invalid %s buffer access: off=%d, var_off=%lld\n",
+			reg_arg_name(env, argno), buf_info, off, var_off);
+		return -EACCES;
+	}
+
+	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 (start > U32_MAX || size < 0 ||
+	    check_add_overflow((u32)start, (u32)size, access_end)) {
+		verbose(env,
+			"%s invalid %s buffer access: off=%lld, size=%d\n",
+			reg_arg_name(env, argno), buf_info, start, size);
+		return -EACCES;
+	}
+
 	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

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

[Shung-Hsi Yu]

Sorry I missed that there was a v2 already. Reposting the comments I
sent to v1 below.

On Mon, Jul 06, 2026 at 11:08:03PM -0700, Sun Jian wrote:
[...]
> @@ -5344,6 +5348,29 @@ static int __check_buffer_access(struct bpf_verifier_env *env,
>  		return -EACCES;
>  	}
>  
> +	var_off = (s64)reg->var_off.value;
> +	if (check_add_overflow(var_off, (s64)off, &start)) {
> +		verbose(env,
> +			"%s invalid %s buffer access: off=%d, var_off=%lld\n",
> +			reg_arg_name(env, argno), buf_info, off, var_off);
> +		return -EACCES;
> +	}

Other pointer offset checks (check_mem_region_access and
adjust_ptr_min_max_vals) seem to check var_off against BPF_MAX_VAR_OFF,
and that `off` being >= 0. It would be great if the convention can be
kept for consistency, albeit being more conservative.

From a blind guess I would guess that compiler does not product a
pattern where `off` encoded in the instruction is negative but the net
overall offset is positive for PTR_TO_TP_BUFFER/PTR_TO_BUF, and
rejecting such pattern is fine.

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

One more comment below.

> +
> +	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 (start > U32_MAX || size < 0 ||
> +	    check_add_overflow((u32)start, (u32)size, access_end)) {
> +		verbose(env,
> +			"%s invalid %s buffer access: off=%lld, size=%d\n",
> +			reg_arg_name(env, argno), buf_info, start, size);
> +		return -EACCES;
> +	}

If we check that var_off is within BPF_MAX_VAR_OFF and that `off` is not
negative, then `var_off + off + size` should be guaranteed to not
overflow max_tp_access, since the maximum of `off` is S16_MAX and the
maximum of `size` is 8.

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

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

[sun jian]

On Tue, Jul 7, 2026 at 3:17 PM Shung-Hsi Yu <shung-hsi.yu@suse.com> wrote:
>
> Sorry I missed that there was a v2 already. Reposting the comments I
> sent to v1 below.
>
> On Mon, Jul 06, 2026 at 11:08:03PM -0700, Sun Jian wrote:
> [...]
> > @@ -5344,6 +5348,29 @@ static int __check_buffer_access(struct bpf_verifier_env *env,
> >               return -EACCES;
> >       }
> >
> > +     var_off = (s64)reg->var_off.value;
> > +     if (check_add_overflow(var_off, (s64)off, &start)) {
> > +             verbose(env,
> > +                     "%s invalid %s buffer access: off=%d, var_off=%lld\n",
> > +                     reg_arg_name(env, argno), buf_info, off, var_off);
> > +             return -EACCES;
> > +     }
>
> Other pointer offset checks (check_mem_region_access and
> adjust_ptr_min_max_vals) seem to check var_off against BPF_MAX_VAR_OFF,
> and that `off` being >= 0. It would be great if the convention can be
> kept for consistency, albeit being more conservative.
>
> From a blind guess I would guess that compiler does not product a
> pattern where `off` encoded in the instruction is negative but the net
> overall offset is positive for PTR_TO_TP_BUFFER/PTR_TO_BUF, and
> rejecting such pattern is fine.
>
>   r6 = *(u64 *)(r1 + 0);
>   r6 += 16;
>   r0 = *(u64 *)(r6 - 8);
>
> One more comment below.
>
> > +
> > +     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 (start > U32_MAX || size < 0 ||
> > +         check_add_overflow((u32)start, (u32)size, access_end)) {
> > +             verbose(env,
> > +                     "%s invalid %s buffer access: off=%lld, size=%d\n",
> > +                     reg_arg_name(env, argno), buf_info, start, size);
> > +             return -EACCES;
> > +     }
>
> If we check that var_off is within BPF_MAX_VAR_OFF and that `off` is not
> negative, then `var_off + off + size` should be guaranteed to not
> overflow max_tp_access, since the maximum of `off` is S16_MAX and the
> maximum of `size` is 8.
>
> > +
> >       return 0;
> >  }
> >
> [...]

Thanks a lot for taking a look!

This makes sense. I’ll rework this to align the buffer access check with
the existing verifier offset convention, including the BPF_MAX_VAR_OFF
bounds, and send a v3.

Thanks again,
Sun Jian

[PATCH bpf v2 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