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

[Kees Cook]

On Tue, Mar 15, 2022 at 09:48:29PM +0000, Edgecombe, Rick P wrote:
> 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?

What I want is to have writers of dynamically-sized arrays able to do
the bounds check in the same place the write happens, so passing "info"
makes sense to me.

-- 
Kees Cook