[PATCH bpf v2] bpf: Check variable-offset accesses against special fields

[PATCH bpf v2] bpf: Check variable-offset accesses against special fields

[Shihuang Liu]

btf_struct_access() rejects direct access to special fields (kptr,
spin_lock, list/rbtree nodes, ...) of program allocated objects, but
for a variable offset it only checks min_off.

With compiler-generated BTF the check at min_off is enough: the
register cannot take every value in [min_off, max_off], since
check_variable_offset() keeps the access inside one array with a
step that is a multiple of the element size, and btf_repeat_fields()
repeats the special fields for every element.  Every reachable
offset therefore selects the same member of some element.

The check breaks for hand-written BTF with overlapping members.
btf_struct_check_meta() only rejects offsets that go backwards, so
this layout is accepted:

  struct node {
          int arr[4];             /* offset 0, size 16 */
          struct foo __kptr *p;   /* offset 8 */
  };

A 4-byte store to arr[i], i in [0, 3], passes the check at offset 0 and
overwrites p at run time when i == 2 or 3; the release path
(bpf_obj_free_fields()) then frees the corrupted pointer.

Check every offset the register can take (min_off + k * step)
against the record.  Constant offsets keep the existing check.
Since only reachable offsets are checked, accesses that stay on a
non-special member of every element (e.g. a store to arr[i].data
with step == sizeof(elem)) keep working.

Fixes: 89e72f81a455 ("bpf: Varying offset access support for PTR_TO_BTF_ID pointers")
Link: https://lore.kernel.org/bpf/20261107135419.81491-1-shlomojune6@gmail.com/
Signed-off-by: Shihuang Liu <shlomojune6@gmail.com>
---
Changes since v1:
- Check only the reachable offsets (min_off + k * step) instead of the
  whole [min_off, max_off + size) range
- Rework the commit message

 kernel/bpf/btf.c | 42 +++++++++++++++++++++++++++++++++++-------
 1 file changed, 35 insertions(+), 7 deletions(-)

diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 8bf39098ba97a..738625d8e5879 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -8056,6 +8056,21 @@ error:
 	return -EINVAL;
 }
 
+static bool btf_field_is_accessed(const struct bpf_reg_state *reg,
+				  s64 min_off, s64 max_off, int size,
+				  const struct btf_field *field)
+{
+	s64 first, last;
+
+	first = max_t(s64, min_off, (s64)field->offset - size + 1);
+	last = min_t(s64, max_off, (s64)field->offset + field->size - 1);
+	if (first > last)
+		return false;
+
+	first += imod(min_off - first, reg->step);
+	return first <= last;
+}
+
 int btf_struct_access(struct bpf_verifier_log *log,
 		      const struct bpf_reg_state *reg,
 		      int _off, int size, enum bpf_access_type atype __maybe_unused,
@@ -8091,15 +8106,28 @@ int btf_struct_access(struct bpf_verifier_log *log,
 		if (!meta)
 			break;
 		rec = meta->record;
+		if (min_off == max_off) {
+			for (i = 0; i < rec->cnt; i++) {
+				struct btf_field *field = &rec->fields[i];
+				u32 offset = field->offset;
+
+				if (off < offset + field->size && offset < off + size) {
+					bpf_log(log, "direct access to %s is disallowed\n",
+						btf_field_type_name(field->type));
+					return -EACCES;
+				}
+			}
+			break;
+		}
+
 		for (i = 0; i < rec->cnt; i++) {
 			struct btf_field *field = &rec->fields[i];
-			u32 offset = field->offset;
-			if (off < offset + field->size && offset < off + size) {
-				bpf_log(log,
-					"direct access to %s is disallowed\n",
-					btf_field_type_name(field->type));
-				return -EACCES;
-			}
+
+			if (!btf_field_is_accessed(reg, min_off, max_off, size, field))
+				continue;
+			bpf_log(log, "direct access to %s is disallowed\n",
+				btf_field_type_name(field->type));
+			return -EACCES;
 		}
 		break;
 	}

Re: [PATCH bpf v2] bpf: Check variable-offset accesses against special fields

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 2233 bytes --]

> bpf: Check variable-offset accesses against special fields

TL;DR: Nit only: the constant-offset loop in btf_struct_access() duplicates
the variable-offset loop, and the review asks whether it could be dropped so
one loop handles both cases.

> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c

[ ... ]

> @@ -8091,15 +8106,28 @@ int btf_struct_access(struct bpf_verifier_log *log,
>  		if (!meta)
>  			break;
>  		rec = meta->record;
> +		if (min_off == max_off) {
> +			for (i = 0; i < rec->cnt; i++) {
> +				struct btf_field *field = &rec->fields[i];
> +				u32 offset = field->offset;
> +
> +				if (off < offset + field->size && offset < off + size) {
> +					bpf_log(log, "direct access to %s is disallowed\n",
> +						btf_field_type_name(field->type));
> +					return -EACCES;
> +				}
> +			}
> +			break;
> +		}
> +
>  		for (i = 0; i < rec->cnt; i++) {
>  			struct btf_field *field = &rec->fields[i];
> -			u32 offset = field->offset;
> -			if (off < offset + field->size && offset < off + size) {
> -				bpf_log(log,
> -					"direct access to %s is disallowed\n",
> -					btf_field_type_name(field->type));
> -				return -EACCES;
> -			}
> +
> +			if (!btf_field_is_accessed(reg, min_off, max_off, size, field))
> +				continue;
> +			bpf_log(log, "direct access to %s is disallowed\n",
> +				btf_field_type_name(field->type));
> +			return -EACCES;
>  		}
>  		break;
>  	}

This isn't a bug, but the two loops over rec->fields[] are near copies of
each other, including the same bpf_log() message.

When min_off == max_off, btf_field_is_accessed() gives the same answer as
the constant-offset loop: first and last reduce to the plain overlap window
around min_off, and imod(min_off - first, reg->step) is imod(0, step), so
the result is just first <= last.

Since btf_field_is_accessed() returns the same result when min_off ==
max_off, could the constant-offset block be dropped so that a single loop
handles both cases?


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/42495726162

Re: [PATCH bpf v2] bpf: Check variable-offset accesses against special fields

[Alexei Starovoitov]

On Sun, Nov 08, 2026 at 07:29 AM Shihuang Liu <shlomojune6@gmail.com> wrote:
> Check every offset the register can take (min_off + k * step)
> against the record.  Constant offsets keep the existing check.

No need for the helper and the second loop.
reg->step is never 0. ___mark_reg_known() sets it to 1 and
reg_bounds_sanity_check() reports 0 as a verifier bug.
So a constant offset is the k == 0 case of the same check.

Something like this (compile tested only):

diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 8bf3909..2265362 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -8094,7 +8094,12 @@ int btf_struct_access(struct bpf_verifier_log *log,
 		for (i = 0; i < rec->cnt; i++) {
 			struct btf_field *field = &rec->fields[i];
 			u32 offset = field->offset;
-			if (off < offset + field->size && offset < off + size) {
+			/* An access at 'lo' or above ends after the start of the field. */
+			s64 lo = max(min_off, (s64)offset - size + 1);
+
+			/* Round up to min_off + k * step. The step is never 0. */
+			lo += imod(min_off - lo, reg->step);
+			if (lo <= max_off && lo < offset + field->size) {
 				bpf_log(log,
 					"direct access to %s is disallowed\n",
 					btf_field_type_name(field->type));


The store to arr[i] from the commit log. It loads today. With the fix
the verifier says "direct access to kptr is disallowed".
clang can't emit such BTF. Compile the struct with p at offset 16.
After __open() move p to offset 8 in bpf_object__btf(), then load.

The store to arr[i].data from my reply to v1. It has to keep loading.

pw-bot: cr

Re: [PATCH bpf v2] bpf: Check variable-offset accesses against special fields

[J4f J4f]

On Mon, 9 Nov 2026 at 00:11, Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Sun, Nov 08, 2026 at 07:29 AM Shihuang Liu <shlomojune6@gmail.com> wrote:
> > Check every offset the register can take (min_off + k * step)
> > against the record.  Constant offsets keep the existing check.
>
> No need for the helper and the second loop.
> reg->step is never 0. ___mark_reg_known() sets it to 1 and
> reg_bounds_sanity_check() reports 0 as a verifier bug.
> So a constant offset is the k == 0 case of the same check.
>
> Something like this (compile tested only):
>
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index 8bf3909..2265362 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -8094,7 +8094,12 @@ int btf_struct_access(struct bpf_verifier_log *log,
>                 for (i = 0; i < rec->cnt; i++) {
>                         struct btf_field *field = &rec->fields[i];
>                         u32 offset = field->offset;
> -                       if (off < offset + field->size && offset < off + size) {
> +                       /* An access at 'lo' or above ends after the start of the field. */
> +                       s64 lo = max(min_off, (s64)offset - size + 1);
> +
> +                       /* Round up to min_off + k * step. The step is never 0. */
> +                       lo += imod(min_off - lo, reg->step);
> +                       if (lo <= max_off && lo < offset + field->size) {
>                                 bpf_log(log,
>                                         "direct access to %s is disallowed\n",
>                                         btf_field_type_name(field->type));
>
>
> The store to arr[i] from the commit log. It loads today. With the fix
> the verifier says "direct access to kptr is disallowed".
> clang can't emit such BTF. Compile the struct with p at offset 16.
> After __open() move p to offset 8 in bpf_object__btf(), then load.
>
> The store to arr[i].data from my reply to v1. It has to keep loading.
>
> pw-bot: cr
Thanks for the review and the suggestion.

Sending v3 shortly.

Thanks,
Shihuang