On Mon, Apr 06, 2020 at 06:16:01PM -0700, Alexei Starovoitov wrote:
SNIP
> > + continue;
> > +
> > + /* the 'off' we're looking for is either equal to start
> > + * of this field or inside of this struct
> > + */
> > + if (btf_type_is_struct(mtype)) {
> > + /* our field must be inside that union or struct */
> > + t = mtype;
> > +
> > + /* adjust offset we're looking for */
> > + off -= moff;
> > + goto again;
> > + }
>
> Looks like copy-paste that should be generalized into common helper.
right, I think we could have some common code with btf_struct_access,
but id not want to complicate the change for rfc
>
> > +
> > + bpf_log(log, "struct %s doesn't have struct field at offset %d\n", tname, off);
> > + return -EACCES;
> > + }
> > +
> > + bpf_log(log, "struct %s doesn't have field at offset %d\n", tname, off);
> > + return -EACCES;
> > +}
> > +
> > static int __btf_resolve_helper_id(struct bpf_verifier_log *log, void *fn,
> > int arg)
> > {
> > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> > index 04c6630cc18f..6eb88bef4379 100644
> > --- a/kernel/bpf/verifier.c
> > +++ b/kernel/bpf/verifier.c
> > @@ -3103,6 +3103,18 @@ static int check_ptr_to_btf_access(struct bpf_verifier_env *env,
> > return 0;
> > }
> >
> > +static void check_ptr_in_btf(struct bpf_verifier_env *env,
> > + struct bpf_reg_state *reg,
> > + u32 exp_id)
> > +{
> > + const struct btf_type *t = btf_type_by_id(btf_vmlinux, reg->btf_id);
> > +
> > + if (!btf_struct_address(&env->log, t, reg->off, exp_id)) {
> > + reg->btf_id = exp_id;
> > + reg->off = 0;
>
> This doesn't look right.
> If you simply overwrite btf_id and off in the reg it will contain wrong info
> in any subsequent instruction.
> Typically it would be ok, since this reg is a function argument and will be
> scratched after the call, but consider:
> bpf_foo(&file->f_path, &file->f_owner);
> The same base register will be used to construct R1 and R2
> and above re-assign will screw up R1.
ok.. I'll use the 'new btf id' just to do check on the helper args
and not change the reg's btf id
thanks,
jirka
