On Fri 21-06-19 10:57:35, Alexander Potapenko wrote: > On Fri, Jun 21, 2019 at 9:09 AM Michal Hocko <mhocko@xxxxxxxxxx> 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
