Re: [PATCH bpf-next 2/8] bpf: Allow trusted args to walk struct when checking BTF IDs

[Alexei Starovoitov]

On Fri, Jan 20, 2023 at 7:26 AM David Vernet <void@xxxxxxxxxxxxx> wrote:
> > >
> > > Yes. Agree. I used unfortunate example in the previous reply with nf_conn___init.
> > > I meant to say:
> > >
> > >  For definition:
> > >  struct nf_conn_init {
> > >     struct nf_conn ct;
> > >  };
> > >  if a kfunc accepts a pointer to nf_conn it should always accept a pointer to nf_conn_init
> > >  for both read and write, because in C that's valid and safe type cast.
> > >
> > > Meainng that C rules apply.
> > > Our triple underscore is special, because it's the "same type".
> > > In the 2nd part of my reply I'm proposing to use the whole suffix "___init" to indicate that.
> > > I think you're arguing that just "___" part is enough to enforce strict match.
> > > Matching foo___flavor with foo should not be allowed.
> > > While passing struct foo_flavor {struct foo;} into a kfunc that accepts 'struct foo'
> > > is safe.
> > > If so, I'm fine with such approach.
> >
> > Alright, I'll spin v2 to treat any type with name___.* as a disallowed
> > alias, and update the documentation to mention it. I was originally
> > going to push back and say that we should just use a single alias like
> > __nocast to keep things simple, but it doesn't feel generalizable
> > enough.
>
> On second thought, unless you guys feel strongly, I'll just check
> ___init. The resulting code is going to be a lot of tricky string
> manipulation / math otherwise. Not _terrible_, but I'd prefer to avoid
> adding it until we have a concrete use-case. And I expect this could be
> implemented much simpler using something like tags, once gcc has support
> for it.

There is bpf_core_is_flavor_sep() that makes it easy to check,
but thinking more about it we probably should stick to strict "___init"
suffix for now, since flavors can appear in progs too and they
are equivalent to corresponding types in the kernel.
The nf_conn___init is kernel only type.
The verifier sees that bpf_xdp_ct_alloc kfunc returns it,
but when we export this kfunc to bpf prog it returns nf_conn.
We probably should pick some other suffix without "___" to distinguish
from flavors. bpf prog should not use nf_conn___init.