Subject: Re: [PATCH v10 bpf-next 08/14] bpf: Add btf_struct_ids_match function

From: Jiri Olsa <[email-masked]>
Date: Sun, 9 Aug 2020 16:47:06 +0200

On Fri, Aug 07, 2020 at 01:04:26PM -0700, Andrii Nakryiko wrote:

SNIP

> > +                               }
> >                         }
> >                 } else if (!fn->check_btf_id(reg->btf_id, arg)) {
> 
> Put this on a wishlist for now. I don't think we should expect
> fb->check_btf_id() to do btf_struct_ids_match() internally, so to
> support this, we'd have to call fb->check_btf_id() inside the loop
> while doing WALK_STRUCT struct. But let's not change all this in this
> patch set, it's involved enough already.
> 
> >                         verbose(env, "Helper does not support %s in R%d\n",
> > @@ -3977,7 +3982,8 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg,
> >
> >                         return -EACCES;
> >                 }
> > -               if (!tnum_is_const(reg->var_off) || reg->var_off.value || reg->off) {
> > +               if (!ids_match &&
> > +                   (!tnum_is_const(reg->var_off) || reg->var_off.value || reg->off)) {
> 
> Isn't this still wrong? if ids_match, but reg->var_off is non-zero,
> that's still bad, right?
> ids_match just "mitigates" reg->off check, so should be something like this:
> 
> if ((reg->off && !ids_match) || !tnum_is_const(reg->var_off) ||
> reg->var_off.value)
>  ... then bad ...

damn you're right, those are separated things,
I mixed it up, I'll send new version

thanks,
jirka