On Thu, Jul 3, 2025 at 2:25 PM Eduard Zingerman <eddyz87@xxxxxxxxx> wrote:
>
> On Wed, 2025-07-02 at 20:18 -0700, Alexei Starovoitov wrote:
> > On Wed, Jul 2, 2025 at 3:42 PM Eduard Zingerman <eddyz87@xxxxxxxxx> 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);
> > > + if (kern_type_id < 0)
> > > + return kern_type_id;
> > > +
> > > + sub->args[i].arg_type = ARG_PTR_TO_BTF_ID | PTR_UNTRUSTED;
> > > + sub->args[i].btf_id = kern_type_id;
> > > + continue;
> > > + }
> >
> > Looking at this hunk standalone (without patch 7) one might get
> > an impression that odd ptr_to_btf_id is allowed that points
> > to non-struct type,
> > but patch 7 sort-of fixes it by handling primitive types first.
> >
> > Still, I think it would be good to add a check here that kern_type_id
> > is a struct kind.
>
> I'm adding this check, but it will go w/o a test:
> - unions are allowed by btf_struct_walk, so need to be accepted
Of course, by "checking a struct kind" I meant btf_type_is_struct()
which does kind == BTF_KIND_STRUCT || kind == BTF_KIND_UNION.
> - function types are anonymous and candidates search wants types with names
> - float -- no candidate in kernel btf
> - func/var/datasec -- need a corrupt BTF to sneak these in.
You're probably right, but extra "if (btf_type_is_struct(..."
just to be safe is imo worth it. syzbot-s and such.
