Re: [PATCH bpf] bpf: Reject writes through untrusted allocated pointers

[Kumar Kartikeya Dwivedi]

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 <memxor@xxxxxxxxx> 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/20260726021304.97ED91F000E9@xxxxxxxxxxxxxxx/
>> > Assisted-by: Codex:gpt-5.6-sol
>> > Signed-off-by: Ning Ding <dingning04@xxxxxxxxx>
>> > ---
>>
>> 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";
>>