linux-kernel - Re: [PATCH v1] mm: initialize struct page for reserved pages in ZONE

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]

Date:   Fri, 22 Jun 2018 06:55:11 +0000
From:   Naoya Horiguchi <n-horiguchi@...jp.nec.com>
To:     Dan Williams <dan.j.williams@...el.com>
CC:     Linux MM <linux-mm@...ck.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Michal Hocko <mhocko@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        "Ingo Molnar" <mingo@...hat.com>, "H. Peter Anvin" <hpa@...or.com>,
        Dave Hansen <dave.hansen@...el.com>
Subject: Re: [PATCH v1] mm: initialize struct page for reserved pages in
 ZONE_DEVICE

On Thu, Jun 21, 2018 at 11:12:01PM -0700, Dan Williams wrote:
> On Thu, Jun 21, 2018 at 11:08 PM, Naoya Horiguchi
> <n-horiguchi@...jp.nec.com> wrote:
> > Reading /proc/kpageflags for pfns allocated by pmem namespace triggers
> > kernel panic with a message like "BUG: unable to handle kernel paging
> > request at fffffffffffffffe".
> >
> > The first few pages (controlled by altmap passed to memmap_init_zone())
> > in the ZONE_DEVICE can skip struct page initialization, which causes
> > the reported issue.
> >
> > This patch simply adds some initialization code for them.
> >
> > Fixes: 4b94ffdc4163 ("x86, mm: introduce vmem_altmap to augment vmemmap_populate()")
> > Signed-off-by: Naoya Horiguchi <n-horiguchi@...jp.nec.com>
> > ---
> >  mm/page_alloc.c | 10 +++++++++-
> >  1 file changed, 9 insertions(+), 1 deletion(-)
> >
> > diff --git v4.17-mmotm-2018-06-07-16-59/mm/page_alloc.c v4.17-mmotm-2018-06-07-16-59_patched/mm/page_alloc.c
> > index 1772513..0b36afe 100644
> > --- v4.17-mmotm-2018-06-07-16-59/mm/page_alloc.c
> > +++ v4.17-mmotm-2018-06-07-16-59_patched/mm/page_alloc.c
> > @@ -5574,8 +5574,16 @@ void __meminit memmap_init_zone(unsigned long size, int nid, unsigned long zone,
> >          * Honor reservation requested by the driver for this ZONE_DEVICE
> >          * memory
> >          */
> > -       if (altmap && start_pfn == altmap->base_pfn)
> > +       if (altmap && start_pfn == altmap->base_pfn) {
> > +               unsigned long i;
> > +
> > +               for (i = 0; i < altmap->reserve; i++) {
> > +                       page = pfn_to_page(start_pfn + i);
> > +                       __init_single_page(page, start_pfn + i, zone, nid);
> > +                       SetPageReserved(page);
> > +               }
> >                 start_pfn += altmap->reserve;
> > +       }
> 
> No, unfortunately this will clobber metadata that lives in that
> reserved area, see __nvdimm_setup_pfn().

Hi Dan,

This patch doesn't touch the reserved region itself, but only
struct pages on the region. I'm still not sure why it's necessary
to leave these struct pages uninitialized for pmem operation?

My another related concern is about memory_failure_dev_pagemap().
If a memory error happens on the reserved pfn range, this function
seems to try to access to the uninitialized struct page and maybe
trigger oops. So do we need something to prevent this?

Thanks,
Naoya Horiguchi