Re: [PATCH bpf-next v1 4/8] bpf: attribute __arg_untrusted for global function parameters

[Eduard Zingerman <eddyz87@gmail.com>]

On Fri, 2025-07-04 at 22:20 +0200, Kumar Kartikeya Dwivedi wrote:
> On Fri, 4 Jul 2025 at 22:06, Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
> > 
> > On Fri, Jul 4, 2025 at 12:23 PM Eduard Zingerman <eddyz87@gmail.com> wrote:
> > > 
> > > On Fri, 2025-07-04 at 21:15 +0200, Kumar Kartikeya Dwivedi wrote:
> > > 
> > > [...]
> > > 
> > > > Yeah, so if the user specifies a type and has co-re enabled, they're
> > > > accessing a kernel struct.
> > > > If they're doing it without co-re, it's broken today already, or they
> > > > know the struct is fixed in layout somehow so it's ok.
> > > > If not, they want to access things at fixed offsets. So we can just
> > > > use the type they're using to model untrusted derefs.
> > > > 
> > > > So always using prog BTF makes sense to me.
> > > 
> > > Ok, I'm switching to always using prog BTF.
> > 
> > Hold on. The concept of ptr_to_btf_id|untrusted that points to
> > prog type doesn't exist today. We should be careful when introducing
> > such things.
> > I prefer to keep btf_get_ptr_to_btf_id() in this patch
> > and think through untrusted|ptr_to prog type later,
> > since the use case of untrusted local type doesn't quite resonate with me.
> 
> Yeah, we can add it separately from this set, but otherwise I don't
> see the problem with the idea.
> There is no reason to restrict ourselves to kernel types.
> All accesses will be untrusted, it's like probe_read so it should be
> well-formed for any type.
> It's the same reason why pointers to non-struct makes sense. Ideally
> any type should be allowed.
> 
> Otherwise to reconstruct a walk of untrusted pointer chains the user
> will do it by hand.
> Showing the structure types to the verifier allows it to be inserted
> automatically.

I agree with Kumar, it's an expansion of the idea behind
bpf_rdonly_cast(...,0). Having some untrusted pointer source
(e.g. value from a program stack trace) one can already write:

  struct foo {
    struct bar *bar;
  };

  struct foo *foo = bpf_rdonly_cast(magic_value, 0);
  struct bar *bar = bpf_rdonly_cast(foo->bar, 0);

We can make it more convenient by introducing an additional kfunc:

  struct foo {
    struct bar *bar;
  };

  struct foo *foo = bpf_rdonly_cast_local(magic_value, bpf_core_type_id_local(struct foo));
  ... just use foo->bar ...

As for global function parameters. Discussed this with Kumar off-list
a bit. There are types with generic names in vmlinux.h, like "region",
"regex", "hash", "key". So, imo, the above bpf_rdonly_cast_local()
should be accompanied by its own tag: __arg_untrusted_local.