[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAG_fn=Vhn4x_wVcftQUC4wh4JOgy8budA4+jj=dnRpPwqEz2TA@mail.gmail.com>
Date: Fri, 21 Jun 2019 11:18:06 +0200
From: Alexander Potapenko <glider@...gle.com>
To: Michal Hocko <mhocko@...nel.org>
Cc: Andrew Morton <akpm@...ux-foundation.org>, Christoph Lameter <cl@...ux.com>,
Kees Cook <keescook@...omium.org>, Masahiro Yamada <yamada.masahiro@...ionext.com>,
James Morris <jmorris@...ei.org>, "Serge E. Hallyn" <serge@...lyn.com>,
Nick Desaulniers <ndesaulniers@...gle.com>, Kostya Serebryany <kcc@...gle.com>,
Dmitry Vyukov <dvyukov@...gle.com>, Sandeep Patil <sspatil@...roid.com>,
Laura Abbott <labbott@...hat.com>, Randy Dunlap <rdunlap@...radead.org>, Jann Horn <jannh@...gle.com>,
Mark Rutland <mark.rutland@....com>, Marco Elver <elver@...gle.com>,
Linux Memory Management List <linux-mm@...ck.org>,
linux-security-module <linux-security-module@...r.kernel.org>,
Kernel Hardening <kernel-hardening@...ts.openwall.com>
Subject: Re: [PATCH v7 1/2] mm: security: introduce init_on_alloc=1 and
init_on_free=1 boot options
On Fri, Jun 21, 2019 at 11:12 AM Michal Hocko <mhocko@...nel.org> wrote:
>
> On Fri 21-06-19 10:57:35, Alexander Potapenko wrote:
> > On Fri, Jun 21, 2019 at 9:09 AM Michal Hocko <mhocko@...nel.org> wrote:
> [...]
> > > > diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
> > > > index fd5c95ff9251..2f75dd0d0d81 100644
> > > > --- a/kernel/kexec_core.c
> > > > +++ b/kernel/kexec_core.c
> > > > @@ -315,7 +315,7 @@ static struct page *kimage_alloc_pages(gfp_t gfp_mask, unsigned int order)
> > > > arch_kexec_post_alloc_pages(page_address(pages), count,
> > > > gfp_mask);
> > > >
> > > > - if (gfp_mask & __GFP_ZERO)
> > > > + if (want_init_on_alloc(gfp_mask))
> > > > for (i = 0; i < count; i++)
> > > > clear_highpage(pages + i);
> > > > }
> > >
> > > I am not really sure I follow here. Why do we want to handle
> > > want_init_on_alloc here? The allocated memory comes from the page
> > > allocator and so it will get zeroed there. arch_kexec_post_alloc_pages
> > > might touch the content there but is there any actual risk of any kind
> > > of leak?
> > You're right, we don't want to initialize this memory if init_on_alloc is on.
> > We need something along the lines of:
> > if (!static_branch_unlikely(&init_on_alloc))
> > if (gfp_mask & __GFP_ZERO)
> > // clear the pages
> >
> > Another option would be to disable initialization in alloc_pages() using a flag.
>
> Or we can simply not care and keen the code the way it is. First of all
> it seems that nobody actually does use __GFP_ZERO unless I have missed
> soemthing
> - kimage_alloc_pages(KEXEC_CONTROL_MEMORY_GFP, order); # GFP_KERNEL | __GFP_NORETRY
> - kimage_alloc_pages(gfp_mask, 0);
> - kimage_alloc_page(image, GFP_KERNEL, KIMAGE_NO_DEST);
> - kimage_alloc_page(image, GFP_HIGHUSER, maddr);
>
> but even if we actually had a user do we care about double intialization
> for something kexec related? It is not any hot path AFAIR.
Yes, sounds good. Spraying the code with too many checks for
init_on_alloc doesn't really look nice.
> --
> Michal Hocko
> SUSE Labs
--
Alexander Potapenko
Software Engineer
Google Germany GmbH
Erika-Mann-Straße, 33
80636 München
Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
