bpf: Reject negative const offsets for buffer pointers

Fixes 022ac0750883 ("bpf: use reg->var_off instead of reg->off for pointers")

kernel/bpf/verifier.c +9 -1

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

kernel/bpf/verifier.c +1 -0

+
 	return 0;
 }
 

On Wed, Jul 08, 2026 at 02:01:50AM -0700, Sun Jian wrote:
[...]

Quoted 1 line

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.

[...]

Quoted 28 lines

kernel/bpf/verifier.c +9 -1

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

kernel/bpf/verifier.c static int __check_buffer_access(struct bpf_verifier_env *env, · line 5364

Quoted 10 lines

kernel/bpf/verifier.c +10 -0

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

kernel/bpf/verifier.c static int __check_buffer_access(struct bpf_verifier_env *env, · line 5370

Quoted 6 lines

kernel/bpf/verifier.c +6 -0

+
+     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 <[email protected]>

Review static int __check_buffer_access(struct bpf_verifier_env *env, · line 5372

Quoted 2 lines

(quoted diff) +2 -0

+
+     *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().

Quoted 4 lines

kernel/bpf/verifier.c +1 -0

+
      return 0;
 }
 

[...]

Quoted 41 lines

Quoted 28 lines

(quoted diff) +9 -1

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

Review static int __check_buffer_access(struct bpf_verifier_env *env, · line 5364

Quoted 10 lines

(quoted diff) +10 -0

+             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

Quoted 6 lines

  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.

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.

On Wed, Jul 08, 2026 at 10:11:01PM +0800, sun jian wrote:
[...]

Quoted 41 lines

Quoted 41 lines

Quoted 28 lines

(quoted diff) +9 -1

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

Review static int __check_buffer_access(struct bpf_verifier_env *env, · line 5364

Quoted 10 lines

(quoted diff) +10 -0

+             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

Quoted 6 lines

Quoted 6 lines

  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.

[...]

Quoted 3 lines

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.

---

kernel/bpf/verifier.c +0 -0

index d46f7db20d8f..e116b33ad83e 100644

kernel/bpf/verifier.c +1 -0

        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;

On Wed, Jul 08, 2026 at 10:11:01PM +0800, sun jian wrote:
[...]

Quoted 41 lines

Quoted 41 lines

Quoted 28 lines

(quoted diff) +9 -1

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

Review static int __check_buffer_access(struct bpf_verifier_env *env, · line 5364

Quoted 10 lines

(quoted diff) +10 -0

+             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

Quoted 6 lines

Quoted 6 lines

  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.

[...]

Quoted 3 lines

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.

Quoted 68 lines · Shung-Hsi Yu

On Wed, Jul 08, 2026 at 10:11:01PM +0800, sun jian wrote:
[...]

Quoted 41 lines

Quoted 41 lines

Quoted 28 lines

(quoted diff) +9 -1

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

Review static int __check_buffer_access(struct bpf_verifier_env *env, · line 5364

Quoted 10 lines

(quoted diff) +10 -0

+             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

Quoted 6 lines

Quoted 6 lines

  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.

[...]

Quoted 3 lines

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.

Quoted 1 line

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.

[...]

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:

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

[...]

Quoted 5 lines

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:

Quoted 6 lines

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?

[...]

Quoted 10 lines · Eduard Zingerman

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

[...]

Quoted 5 lines

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:

Quoted 8 lines

Quoted 6 lines

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

Quoted 2 lines

[...]

Quoted 2 lines · Eduard Zingerman

Quoted 1 line · Shung-Hsi Yu

On Wed, Jul 08, 2026 at 10:11:01PM +0800, sun jian wrote:

Quoted 11 lines

Quoted 11 lines

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.

Quoted 5 lines

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.

Quoted 12 lines

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

Quoted 7 lines

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.

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;

[...]

Quoted 3 lines · sun jian

Quoted 2 lines · Eduard Zingerman

Quoted 1 line · Shung-Hsi Yu

On Wed, Jul 08, 2026 at 10:11:01PM +0800, sun jian wrote:

[...]

Quoted 11 lines

Quoted 11 lines

Quoted 11 lines

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.

[...]

Quoted 5 lines

Quoted 5 lines

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.

Quoted 7 lines

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:

Quoted 6 lines

#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;

Quoted 19 lines

Quoted 19 lines

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.

[...]

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.

On Fri, Jul 10, 2026 at 06:27:41PM +0800, sun jian wrote:
[...]

Quoted 9 lines

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

Quoted 5 lines

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.

Quoted 10 lines

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.

	var_off = (s64)reg->var_off.value;
	if (var_off >= BPF_MAX_VAR_OFF || var_off <= -BPF_MAX_VAR_OFF)

	start = var_off + off;
	if (start < 0)

	if (size < 0) {

Quoted 54 lines · Shung-Hsi Yu

On Fri, Jul 10, 2026 at 06:27:41PM +0800, sun jian wrote:
[...]

Quoted 9 lines

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

Quoted 5 lines

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.

Quoted 10 lines

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

Quoted 2 lines

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:

Quoted 2 lines

	var_off = (s64)reg->var_off.value;
	if (var_off >= BPF_MAX_VAR_OFF || var_off <= -BPF_MAX_VAR_OFF)

This should hold already.

Quoted 2 lines

	start = var_off + off;
	if (start < 0)

This one is necessary.

Quoted 1 line

	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()).

Quoted 4 lines

Quoted 2 lines

	var_off = (s64)reg->var_off.value;
	if (var_off >= BPF_MAX_VAR_OFF || var_off <= -BPF_MAX_VAR_OFF)

This should hold already.

FWIW, I had thought so, but after checking I realize that the verifier only ensure that

-BPF_MAX_VAR_OFF < smin < BPF_MAX_VAR_OFF

holds true in check_reg_sane_offset_{scalar,ptr}(), and does not check smax, so technically var_off.value can be greater than BPF_MAX_VAR_OFF when we reach here...

..., just that it doesn't really matter much as it is rejected anyway. And if it is a constant then smin=smax, so for a constant value it does always falls between +-BPF_MAX_VAR_OFF.

  0: (79) r6 = *(u64 *)(r1 +0)          ; R1=ctx() R6=tp_buffer()
  1: (85) call bpf_get_prandom_u32#7    ; R0=scalar()
  2: (bf) r2 = r0                       ; R0=scalar(id=1) R2=scalar(id=1)
  3: (67) r2 <<= 32                     ; R2=scalar(smax=0x7fffffff00000000,smin32=0,smax32=umax32=0,var_off=(0x0; 0xffffffff00000000))
  4: (77) r2 >>= 32                     ; R2=scalar(smin=0,smax=umax=0xffffffff,var_off=(0x0; 0xffffffff))
  5: (07) r2 += 536870911               ; R2=scalar(smin=umin=0x1fffffff,smax=umax=0x11ffffffe,var_off=(0x0; 0x1ffffffff))
                /* r2 is now [2^29 - 1, 2^32 + 2^29 - 2] */

6: (0f) r6 += r2
7: R2=scalar(smin=umin=0x1fffffff,smax=umax=0x11ffffffe,var_off=(0x0; 0x1ffffffff)) R6=tp_buffer(smin=umin=0x1fffffff,smax=umax=0x11ffffffe,var_off=(0x0; 0x1ffffffff)) 7: (79) r0 = *(u64 *)(r6 +0)
R6 invalid variable buffer offset: off=0, var_off=(0x0; 0x1ffffffff)

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 <[email protected]>

---
 .../raw_tp_writable_reject_nbd_invalid.c      | 58 +++++++++++-------- .../bpf/progs/verifier_raw_tp_writable.c      | 16 +++++

 2 files changed, 51 insertions(+), 23 deletions(-)

tools/testing/selftests/bpf/prog_tests/raw_tp_writable_reject_nbd_invalid.c +0 -0

index 216b0dfac0fe..efe4cad47b28 100644
@@ -4,12 +4,31 @@
  #include <linux/nbd.h>
  #include "bpf_util.h"

tools/testing/selftests/bpf/prog_tests/raw_tp_writable_reject_nbd_invalid.c +2 -1

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

 {

tools/testing/selftests/bpf/prog_tests/raw_tp_writable_reject_nbd_invalid.c +1 -1

-	__u32 duration = 0;
+	LIBBPF_OPTS(bpf_prog_load_opts, opts);

  char error[4096];

tools/testing/selftests/bpf/prog_tests/raw_tp_writable_reject_nbd_invalid.c +17 -1

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

tools/testing/selftests/bpf/prog_tests/raw_tp_writable_reject_nbd_invalid.c +2 -0

+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),

tools/testing/selftests/bpf/prog_tests/raw_tp_writable_reject_nbd_invalid.c +0 -0

@@ -19,25 +38,18 @@ void test_raw_tp_writable_reject_nbd_invalid(void)
  		BPF_EXIT_INSN(),
  	};

tools/testing/selftests/bpf/prog_tests/raw_tp_writable_reject_nbd_invalid.c +9 -17

-	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(),
+	};

tools/testing/selftests/bpf/prog_tests/raw_tp_writable_reject_nbd_invalid.c +4 -3

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

 }