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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <202203151946.0ECF6FC@keescook>
Date:   Tue, 15 Mar 2022 19:48:05 -0700
From:   Kees Cook <keescook@...omium.org>
To:     "Edgecombe, Rick P" <rick.p.edgecombe@...el.com>
Cc:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "Yu, Yu-cheng" <yu-cheng.yu@...el.com>,
        "viro@...iv.linux.org.uk" <viro@...iv.linux.org.uk>,
        "Williams, Dan J" <dan.j.williams@...el.com>,
        "Wysocki, Rafael J" <rafael.j.wysocki@...el.com>,
        "ebiederm@...ssion.com" <ebiederm@...ssion.com>,
        "Chatre, Reinette" <reinette.chatre@...el.com>,
        "linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>,
        "Luck, Tony" <tony.luck@...el.com>,
        "Hansen, Dave" <dave.hansen@...el.com>,
        "Brown, Len" <len.brown@...el.com>
Subject: Re: [PATCH 3/3] elf: Don't write past end of notes for regset gap

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