[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Fri, 21 Jun 2019 11:11:59 +0200
From: Michal Hocko <mhocko@...nel.org>
To: Alexander Potapenko <glider@...gle.com>
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 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.
--
Michal Hocko
SUSE Labs
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.
