* [PATCH] bpf: fix special field check with variable offset access
@ 2026-11-07 13:54 Shihuang Liu
2026-11-07 14:35 ` bot+bpf-ci
2026-11-07 16:58 ` Alexei Starovoitov
0 siblings, 2 replies; 4+ messages in thread
From: Shihuang Liu @ 2026-11-07 13:54 UTC (permalink / raw)
To: bpf
Cc: ast, daniel, andrii, eddyz87, memxor, martin.lau, song,
yonghong.song, jolsa, emil, ihor.solodrai, Shihuang Liu
btf_struct_access() rejects direct access to special fields (kptr,
spin_lock, list/rbtree nodes, ...) of program allocated objects, but
for variable offset access the check is done at min_off only, while
the whole [min_off, max_off] range can be reached at runtime. A read
leaks the pointer; a write corrupts the refcounted pointer and crashes
the release path (bpf_obj_free_fields -> bpf_mem_free_rcu).
Check the whole [min_off, max_off + size) range against the record,
like check_map_access() already does for map values.
Fixes: 89e72f81a455 ("bpf: Varying offset access support for PTR_TO_BTF_ID pointers")
Signed-off-by: Shihuang Liu <shlomojune6@gmail.com>
---
kernel/bpf/btf.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 8bf39098ba97a..47a64a57b370d 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -8094,7 +8094,8 @@ 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) {
+ if (min_off < (s64)offset + field->size &&
+ (s64)offset < max_off + size) {
bpf_log(log,
"direct access to %s is disallowed\n",
btf_field_type_name(field->type));
--
2.43.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] bpf: fix special field check with variable offset access
2026-11-07 13:54 [PATCH] bpf: fix special field check with variable offset access Shihuang Liu
@ 2026-11-07 14:35 ` bot+bpf-ci
2026-11-07 16:58 ` Alexei Starovoitov
1 sibling, 0 replies; 4+ messages in thread
From: bot+bpf-ci @ 2026-11-07 14:35 UTC (permalink / raw)
To: shlomojune6, bpf
Cc: ast, daniel, andrii, eddyz87, memxor, martin.lau, song,
yonghong.song, jolsa, emil, ihor.solodrai, shlomojune6, ast,
andrii, daniel, martin.lau, eddyz87, yonghong.song, mason,
ihor.solodrai
[-- Attachment #1: Type: text/plain, Size: 4102 bytes --]
> bpf: fix special field check with variable offset access
TL;DR: The wider special-field check in btf_struct_access() may reject safe
variable-offset array accesses (e.g. arr[i].val next to kptrs) that loaded
before; no reproducer or selftest for the claimed bug.
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
[ ... ]
> @@ -8094,7 +8094,8 @@ 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) {
> + if (min_off < (s64)offset + field->size &&
> + (s64)offset < max_off + size) {
Does this reject variable offset accesses that were safe before, such as
p->arr[i].field where the array element type also holds a special field?
The step check in check_variable_offset() only accepts the access if
reg->step is a multiple of the array element size and the access stays inside
the array:
if (esize == 0 || reg->step % esize != 0)
continue;
...
if (check_add_overflow(array_start, (s64)asize, &array_end) ||
check_add_overflow(max_off, (s64)size, &access_end) ||
access_end > array_end)
continue;
return ret;
reg_bounds_sync() keeps the register on the base + step * k line, so the
offsets reachable at runtime are min_off + k * step, not every value in
[min_off, max_off]. Each of them sits at the same position inside its array
element as min_off does inside element 0. Special fields of an array of
structs are repeated with the element stride by btf_find_nested_struct() ->
btf_repeat_fields(..., nelems - 1, t->size), so the old test at min_off
already decided whether a special field is touched in every element.
For example, a program that was previously accepted:
struct elem { long val; struct node_data __kptr *p; }; /* 16 bytes */
struct holder { struct bpf_rb_node rb; struct elem arr[4]; };
o = bpf_obj_new(struct holder);
/* r1 = o + i * 16, i in [0, 3] -> step 16 */
v = *(u64 *)(r1 + offsetof(struct holder, arr[0].val));
Here min_off = 32 and max_off = 80, so the reads are at 32, 48, 64 and 80,
which are all arr[k].val. The old test sees no overlap: [32, 40) against the
kptrs at 40, 56, 72 and 88. check_variable_offset() accepts it because
16 % 16 == 0 and access_end 88 <= array_end 96. The new test checks [32, 88),
finds the kptrs at 40, 56 and 72, and fails with "direct access to kptr is
disallowed". Elements holding BPF_LIST_HEAD or BPF_RB_ROOT, the other kinds
btf_repeat_fields() repeats, are rejected the same way.
The SCEV loop widening from the same series (6ccf49f79327, which allows
PTR_TO_BTF_ID in is_widenable_reg_type()) may turn a plain bounded loop over
o->arr[i].val into exactly this kind of varying offset access. If so,
programs that verified before the series by unrolling with constant offsets
may now fail to load.
The commit message says "the whole [min_off, max_off] range can be reached at
runtime". That does not match the base/step model from 89e72f81a455. I could
not construct a case where the old min_off test is bypassed for a MEM_ALLOC
object: unions are not scanned for special fields, flexible arrays are not
walked for allocated types, and every success path goes through the step
check.
Is there a reproducer for the leak or crash in bpf_obj_free_fields()? If the
real problem is unsound step tracking somewhere, the array bounds part of
check_variable_offset() would be affected too.
If the full range test is wanted anyway, could it be limited to cases where
the step check cannot guarantee the same intra-element offset, or could the
commit message explain why rejecting these accesses is acceptable? A selftest
for the scenario being fixed would also help.
---
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/42380405667
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] bpf: fix special field check with variable offset access
2026-11-07 13:54 [PATCH] bpf: fix special field check with variable offset access Shihuang Liu
2026-11-07 14:35 ` bot+bpf-ci
@ 2026-11-07 16:58 ` Alexei Starovoitov
2026-11-08 15:29 ` J4f J4f
1 sibling, 1 reply; 4+ messages in thread
From: Alexei Starovoitov @ 2026-11-07 16:58 UTC (permalink / raw)
To: Shihuang Liu, bpf
Cc: daniel, andrii, eddyz87, memxor, martin.lau, song, yonghong.song,
jolsa, emil, ihor.solodrai
On Sat, Nov 07, 2026 at 05:54 AM Shihuang Liu <shlomojune6@gmail.com> wrote:
> btf_struct_access() rejects direct access to special fields (kptr,
> spin_lock, list/rbtree nodes, ...) of program allocated objects, but
> for variable offset access the check is done at min_off only, while
> the whole [min_off, max_off] range can be reached at runtime. A read
> leaks the pointer; a write corrupts the refcounted pointer and crashes
> the release path (bpf_obj_free_fields -> bpf_mem_free_rcu).
Not the whole range.
The code at check_variable_offset: keeps the access inside one array.
reg->step has to be a multiple of the element size.
Every access lands on the same member of some element.
btf_repeat_fields() repeats the special fields for every element.
So with BTF from a compiler the check at min_off is enough.
I think it takes hand-made BTF with members that overlap:
struct node {
u64 arr[4]; offset 0, size 32
struct foo __kptr *p; offset 8
};
btf_struct_check_meta() only rejects offsets that go down.
A store to arr[i], i in [0, 3], passes the check at offset 0 and hits p.
Is that how you got there? Explain in commit log.
Also drop "A read leaks the pointer".
check_ptr_to_btf_access() allows these loads with CAP_PERFMON only.
> - if (off < offset + field->size && offset < off + size) {
> + if (min_off < (s64)offset + field->size &&
> + (s64)offset < max_off + size) {
This rejects a prog that loads today:
struct elem { u64 data; struct foo __kptr *p; };
struct node { struct elem arr[4]; struct foo __kptr *q; };
n->arr[i].data = 1; i in [0, 3], step 16
pw-bot: cr
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] bpf: fix special field check with variable offset access
2026-11-07 16:58 ` Alexei Starovoitov
@ 2026-11-08 15:29 ` J4f J4f
0 siblings, 0 replies; 4+ messages in thread
From: J4f J4f @ 2026-11-08 15:29 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: bpf, daniel, andrii, eddyz87, memxor, martin.lau, song,
yonghong.song, jolsa, emil, ihor.solodrai
On Sun, 8 Nov 2026 at 00:58, Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Sat, Nov 07, 2026 at 05:54 AM Shihuang Liu <shlomojune6@gmail.com> wrote:
> > btf_struct_access() rejects direct access to special fields (kptr,
> > spin_lock, list/rbtree nodes, ...) of program allocated objects, but
> > for variable offset access the check is done at min_off only, while
> > the whole [min_off, max_off] range can be reached at runtime. A read
> > leaks the pointer; a write corrupts the refcounted pointer and crashes
> > the release path (bpf_obj_free_fields -> bpf_mem_free_rcu).
Right.
I'll rewrite that part.
> Not the whole range.
> The code at check_variable_offset: keeps the access inside one array.
> reg->step has to be a multiple of the element size.
> Every access lands on the same member of some element.
> btf_repeat_fields() repeats the special fields for every element.
> So with BTF from a compiler the check at min_off is enough.
>
> I think it takes hand-made BTF with members that overlap:
>
> struct node {
> u64 arr[4]; offset 0, size 32
> struct foo __kptr *p; offset 8
> };
>
> btf_struct_check_meta() only rejects offsets that go down.
> A store to arr[i], i in [0, 3], passes the check at offset 0 and hits p.
>
> Is that how you got there? Explain in commit log.
Yes, exactly. The PoC uses
struct evil {
int a[4]; /* offset 0, size 16 */
struct foo __kptr *p; /* offset 8, overlaps a[2..3] */
};
> Also drop "A read leaks the pointer".
> check_ptr_to_btf_access() allows these loads with CAP_PERFMON only.
Will drop it.
> > - if (off < offset + field->size && offset < off + size) {
> > + if (min_off < (s64)offset + field->size &&
> > + (s64)offset < max_off + size) {
>
> This rejects a prog that loads today:
>
> struct elem { u64 data; struct foo __kptr *p; };
> struct node { struct elem arr[4]; struct foo __kptr *q; };
>
> n->arr[i].data = 1; i in [0, 3], step 16
>
> pw-bot: cr
Agreed.
Sending v2 shortly.
Thanks,
Shihuang
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-11-08 15:29 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-07 13:54 [PATCH] bpf: fix special field check with variable offset access Shihuang Liu
2026-11-07 14:35 ` bot+bpf-ci
2026-11-07 16:58 ` Alexei Starovoitov
2026-11-08 15:29 ` J4f J4f
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox