Re: [PATCH 3/3] elf: Don't write past end of notes for regset gap

["Edgecombe, Rick P" <rick.p.edgecombe@intel.com>]

On Tue, 2022-03-15 at 13:37 -0700, Kees Cook wrote:
> >        /*
> >         * Each other regset might generate a note too.  For each
> > regset
> > -      * that has no core_note_type or is inactive, we leave t-
> > >notes[i]
> > -      * all zero and we'll know to skip writing it later.
> > +      * that has no core_note_type or is inactive, skip it.
> >         */
> > -     for (i = 1; i < view->n; ++i) {
> > -             const struct user_regset *regset = &view->regsets[i];
> > +     note_iter = 1;
> > +     for (view_iter = 1; view_iter < view->n; ++view_iter) {
> > +             const struct user_regset *regset = &view-
> > >regsets[view_iter];
> >                int note_type = regset->core_note_type;
> >                bool is_fpreg = note_type == NT_PRFPREG;
> >                void *data;
> > @@ -1800,10 +1800,11 @@ static int fill_thread_core_info(struct
> > elf_thread_core_info *t,
> >                if (is_fpreg)
> >                        SET_PR_FPVALID(&t->prstatus);
> >   
> 
> info->thread_notes contains the count. Since fill_thread_core_info()
> passes a info member by reference, it might make sense to just pass
> info
> itself, then the size can be written and a bounds-check can be added
> here:
> 
>                 if (WARN_ON_ONCE(i >= info->thread_notes))
>                         continue;

Hi Kees,

Thanks for the quick response.

Are you saying in addition to utilizing the allocation better, also
catch if the allocation is still too small? Or do this check instead of
the change in how to utilize the array, and then maintain the
restriction on having gaps in the regsets?

If it's the former, it seems a bit excessive since the allocation and
usage are only one function call away from each other and the logic is
now such that it can't overflow. I can add it if you want.

If it's to just warn on the gaps, it could also be done directly like:
/* Don't expect gaps in regset views */
WARN_ON(!regset->regset_get);

And it might be a little clearer of a hint about this expectation of
the arch's.

Let me know what you prefer and I can make the change.