On Mon Aug 3, 2026 at 4:29 AM CEST, Ning Ding wrote: > The existing PTR_UNTRUSTED branch is after the access-permission > check. It only handles the case where we propagate the PTR_UNTRUSTED > flag to a field pointer loaded through a BPF_READ. For a direct write > such as res->key = 42, no pointer is loaded and that branch has no > effect: > > my_child = res->child; // in that case, my_child will be marked with > PTR_UNTRUSTED, that is what the existing PTR_UNTRUSTED do. > res->key = 42; // The existing check doesn't handle this. >
First, please don't top post. Second, I don't understand your explanation. WDYM by "after the access permission check"? It gets invoked for BPF_WRITE, so should be rejecting res->key = 42 if 'res' is a pointer with PTR_UNTRUSTED flag set. What am I missing? > On Sun, Aug 2, 2026 at 4:32 PM Kumar Kartikeya Dwivedi <[email protected]> > wrote: >> >> On Tue Jul 28, 2026 at 5:38 AM CEST, Ning Ding wrote: >> > A kptr loaded under an RCU read lock has MEM_ALLOC set. After >> > bpf_rcu_read_unlock(), the verifier marks it PTR_UNTRUSTED, but >> > check_ptr_to_btf_access() still allows writes because >> > type_is_ptr_alloc_obj() ignores that flag. >> > >> > Reject writes through MEM_ALLOC pointers that are PTR_UNTRUSTED. Reads >> > remain allowed and are converted to probe-memory accesses. >> > >> > Also skip the owning-reference check for these pointers, since they no >> > longer have a live reference after leaving the RCU critical section. >> > >> > Add verifier tests for the rejected write and permitted read cases. >> > >> > Fixes: 503e4def5414 ("bpf: Replace open code with for allocated object >> > check") >> > Closes: >> > https://lore.kernel.org/all/[email protected]/ >> > Assisted-by: Codex:gpt-5.6-sol >> > Signed-off-by: Ning Ding <[email protected]> >> > --- >> >> Same comment for this one, split kernel and selftests into separate patches. >> >> > kernel/bpf/verifier.c | 6 +- >> > .../bpf/progs/local_kptr_stash_fail.c | 58 +++++++++++++++++++ >> > 2 files changed, 62 insertions(+), 2 deletions(-) >> > >> > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c >> > index 7aa47342dc65..24c151ad2e62 100644 >> > --- a/kernel/bpf/verifier.c >> > +++ b/kernel/bpf/verifier.c >> > @@ -5804,13 +5804,15 @@ static int check_ptr_to_btf_access(struct >> > bpf_verifier_env *env, >> > * program allocated objects (which always have id > 0), >> > * but not for untrusted PTR_TO_BTF_ID | MEM_ALLOC. >> > */ >> > - if (atype != BPF_READ && !type_is_ptr_alloc_obj(reg->type)) { >> > + if (atype != BPF_READ && >> > + (!type_is_ptr_alloc_obj(reg->type) || reg->type & >> > PTR_UNTRUSTED)) { >> > verbose(env, "only read is supported\n"); >> > return -EACCES; >> > } >> >> There is a PTR_UNTRUSTED check already earlier in this function, why is that >> not >> working or not enough? >> >> pw-bot: cr >> >> > >> > if (type_is_alloc(reg->type) && >> > !type_is_non_owning_ref(reg->type) && >> > - !(reg->type & MEM_RCU) && !reg_is_referenced(env, reg)) { >> > + !(reg->type & (MEM_RCU | PTR_UNTRUSTED)) && >> > + !reg_is_referenced(env, reg)) { >> > verifier_bug(env, "allocated object must have a >> > referenced id"); >> > return -EFAULT; >> > } >> > diff --git a/tools/testing/selftests/bpf/progs/local_kptr_stash_fail.c >> > b/tools/testing/selftests/bpf/progs/local_kptr_stash_fail.c >> > index fcf7a7567da2..d7b8cd6458bb 100644 >> > --- a/tools/testing/selftests/bpf/progs/local_kptr_stash_fail.c >> > +++ b/tools/testing/selftests/bpf/progs/local_kptr_stash_fail.c >> > @@ -8,14 +8,22 @@ >> > #include "../bpf_experimental.h" >> > #include "bpf_misc.h" >> > >> > +extern void bpf_rcu_read_lock(void) __ksym; >> > +extern void bpf_rcu_read_unlock(void) __ksym; >> > + >> > struct node_data { >> > long key; >> > long data; >> > struct bpf_rb_node node; >> > }; >> > >> > +struct plain_data { >> > + long key; >> > +}; >> > + >> > struct map_value { >> > struct node_data __kptr *node; >> > + struct plain_data __kptr *plain; >> > }; >> > >> > struct node_data2 { >> > @@ -31,6 +39,7 @@ struct node_data2 { >> > * [35] TYPE_TAG 'kptr_ref' type_id=34 >> > */ >> > struct node_data *just_here_because_btf_bug; >> > +struct plain_data *just_here_because_btf_bug2; >> > >> > struct { >> > __uint(type, BPF_MAP_TYPE_ARRAY); >> > @@ -82,4 +91,53 @@ long drop_rb_node_off(void *ctx) >> > return 0; >> > } >> > >> > +SEC("?tc") >> > +__failure __msg("only read is supported") >> > +long write_untrusted_alloc_obj(void *ctx) >> > +{ >> > + struct map_value *mapval; >> > + struct node_data *res; >> > + int idx = 0; >> > + >> > + mapval = bpf_map_lookup_elem(&some_nodes, &idx); >> > + if (!mapval) >> > + return 1; >> > + >> > + bpf_rcu_read_lock(); >> > + res = mapval->node; >> > + if (!res) { >> > + bpf_rcu_read_unlock(); >> > + return 2; >> > + } >> > + barrier_var(res); >> > + bpf_rcu_read_unlock(); >> > + >> > + res->key = 42; >> > + return 0; >> > +} >> > + >> > +SEC("?tc") >> > +__success >> > +long read_untrusted_alloc_obj(void *ctx) >> > +{ >> > + struct map_value *mapval; >> > + struct plain_data *res; >> > + int idx = 0; >> > + >> > + mapval = bpf_map_lookup_elem(&some_nodes, &idx); >> > + if (!mapval) >> > + return 1; >> > + >> > + bpf_rcu_read_lock(); >> > + res = mapval->plain; >> > + if (!res) { >> > + bpf_rcu_read_unlock(); >> > + return 2; >> > + } >> > + barrier_var(res); >> > + bpf_rcu_read_unlock(); >> > + >> > + return res->key; >> > +} >> > + >> > char _license[] SEC("license") = "GPL"; >>
