On Fri, 4 Jul 2025 at 20:33, Eduard Zingerman <eddyz87@xxxxxxxxx> wrote:
>
> On Fri, 2025-07-04 at 11:28 -0700, Eduard Zingerman wrote:
> > On Fri, 2025-07-04 at 20:03 +0200, Kumar Kartikeya Dwivedi wrote:
> >
> > [...]
> >
> > > > @@ -7818,6 +7821,22 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog)
> > > > sub->args[i].btf_id = kern_type_id;
> > > > continue;
> > > > }
> > > > + if (tags & ARG_TAG_UNTRUSTED) {
> > > > + int kern_type_id;
> > > > +
> > > > + if (tags & ~ARG_TAG_UNTRUSTED) {
> > > > + bpf_log(log, "arg#%d untrusted cannot be combined with any other tags\n", i);
> > > > + return -EINVAL;
> > > > + }
> > > > +
> > > > + kern_type_id = btf_get_ptr_to_btf_id(log, i, btf, t);
> > >
> > > So while this makes sense for trusted, I think for untrusted, we
> > > should allow types in program BTF as well.
> > > This is one of the things I think lacks in bpf_rdonly_cast as well, to
> > > be able to cast to types in program BTF.
> > > Say you want to reinterpret some kernel memory into your own type and
> > > access it using a struct in the program which is a different type.
> > > I think it makes sense to make this work.
> >
> > Hi Kumar,
> >
> > Thank you for the review.
> > Allowing local program BTF makes sense to me.
> > I assume we should first search in kernel BTF and fallback to program
> > BTF if nothing found. This way verifier might catch a program
> > accessing kernel data structure but having wrong assumptions about
> > field offsets (not using CO-RE). On the other hand, this might get
> > confusing if there is an accidental conflict between kernel data
> > structure name and program local data structure name.
>
> Maybe just add __arg_untrusted_local and avoid ambiguity?
That might be less ambiguous, sure. But I don't see why the fallback
would be confusing.
It might be nice if we can support it without asking users to learn
about the difference between the two tags, but if it's too ugly we can
go with explicit local tag.
A user can have a struct without preserve_access_index now and having
the same name as the kernel struct, and the program will load things
at potentially wrong offsets.
If the same type exists, the program would fail compilation in C due
to duplicate types.
Are there any other cases where it might be a footgun that you anticipate?
>
> > Supporting bpf_core_cast for both prog BTF and kernel BTF types is not
> > trivial because we cannot disambiguate local vs kernel types.
> > IIRC module BTF types probably don't work either but that's a different story.
> I can add bpf_rdonly_cast_local() as a followup, do you remember
> context in which you needed this?
Adding Matt.
Not long ago we were discussing iterating over the bpf linked list
since support doesn't exist in the kernel and it was safe in the
specific context to iterate over the list.
Ofcourse, we could add iteration support in the kernel, but another
approach would be the ability to subtract offset from node to arrive
at an untrusted pointer to type in prog BTF (that was allocated using
bpf_obj_new).
But bpf_core_cast didn't work there, so we ended up discarding that approach.
Not to get hung up on this specific example, but I think it would be
useful in general.
