Re: [RESEND][PATCH bpf 1/2] bpf: Check the remaining info_cnt before repeating btf fields

[Eduard Zingerman <eddyz87@gmail.com>]

On Thu, 2024-09-12 at 21:13 +0200, Thinker Li wrote:

[...]

> > > diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> > > index a4e4f8d43ecf..9a4a074d26f5 100644
> > > --- a/kernel/bpf/btf.c
> > > +++ b/kernel/bpf/btf.c
> > > @@ -3592,6 +3592,12 @@ static int btf_find_nested_struct(const struct btf *btf, const struct btf_type *
> > >                info[i].off += off;
> > >   
> > >        if (nelems > 1) {
> > > +             /* The type of struct size or variable size is u32,
> > > +              * so the multiplication will not overflow.
> > > +              */
> > > +             if (ret * nelems > info_cnt)
> > > +                     return -E2BIG;
> > > +
> > >                err = btf_repeat_fields(info, ret, nelems - 1, t->size);
> > >                if (err == 0)
> > >                        ret *= nelems;
> > 
> > 
> > btf_repeat_fields(struct btf_field_info *info,
> >                   u32 field_cnt, u32 repeat_cnt, u32 elem_size)
> > 
> > copies field "field_cnt * repeat_cnt" times,
> > in this case field_cnt == ret, repeat_cnt == nelems - 1,
> > should the check be "ret * (nelems - 1) > info_cnt"?
> > 
> > I suggest to add info_cnt as a parameter of btf_repeat_fields() and do
> > this check there. So that the check won't be forgotten again if
> > btf_repeat_fields() is used elsewhere. Wdyt?
> > 
> 
> Should not this check be moved before the earlier for-loop?

Shouldn't the check for 'ret <= 0' be enough to make sure the for-loop
is fine?