linux-kernel - Re: [PATCH 2/4] mm/sparse: skip no-map memblock check when fill_subsection

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 for Android: free password hash cracker in your pocket

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

Message-ID: <181f0a5f2a5.cf94ce78513585.4158910057206462182@linux.beauty>
Date:   Tue, 12 Jul 2022 12:23:36 +0800
From:   Li Chen <me@...ux.beauty>
To:     "David Hildenbrand" <david@...hat.com>
Cc:     "Catalin Marinas" <catalin.marinas@....com>,
        "Will Deacon" <will@...nel.org>,
        "Rob Herring" <robh+dt@...nel.org>,
        "Frank Rowand" <frowand.list@...il.com>,
        "Andrew Morton" <akpm@...ux-foundation.org>,
        "Li Chen" <lchen@...arella.com>,
        "linux-arm-kernel" <linux-arm-kernel@...ts.infradead.org>,
        "linux-kernel" <linux-kernel@...r.kernel.org>,
        "devicetree" <devicetree@...r.kernel.org>,
        "linux-mm" <linux-mm@...ck.org>
Subject: Re: [PATCH 2/4] mm/sparse: skip no-map memblock check when
 fill_subsection_map

Hi David,
 ---- On Mon, 11 Jul 2022 22:53:36 +0800  David Hildenbrand <david@...hat.com> wrote --- 
 > On 11.07.22 14:24, Li Chen wrote:
 > > From: Li Chen <lchen@...arella.com>
 > > 
 > > When mhp use sparse_add_section, don't check no-map region,
 > > so that to allow no-map reserved memory to get struct page
 > > support.
 > > 
 > > Signed-off-by: Li Chen <lchen@...arella.com>
 > > Change-Id: I0d2673cec1b66adf695251037a00c240976b226f
 > > ---
 > >  mm/sparse.c | 4 +++-
 > >  1 file changed, 3 insertions(+), 1 deletion(-)
 > > 
 > > diff --git a/mm/sparse.c b/mm/sparse.c
 > > index 120bc8ea5293..a29cd1e7014f 100644
 > > --- a/mm/sparse.c
 > > +++ b/mm/sparse.c
 > > @@ -690,7 +690,9 @@ static int fill_subsection_map(unsigned long pfn, unsigned long nr_pages)
 > >  
 > >      if (bitmap_empty(map, SUBSECTIONS_PER_SECTION))
 > >          rc = -EINVAL;
 > > -    else if (bitmap_intersects(map, subsection_map, SUBSECTIONS_PER_SECTION))
 > > +    else if (memblock_is_map_memory(PFN_PHYS(pfn)) &&
 > > +         bitmap_intersects(map, subsection_map,
 > > +                   SUBSECTIONS_PER_SECTION))
 > >          rc = -EEXIST;
 > >      else
 > >          bitmap_or(subsection_map, map, subsection_map,
 > 
 > I'm not sure I follow completely what you are trying to achieve. But if
 > you have to add memblock hacks into mm/sparse.c you're most probably
 > doing something wrong.
 > 
 > Please explain why that change is necessary, and why it is safe.

In the current sparse memory model, free_area_init will insert all memblock.memory into subsection_map and no-map rmem is also a 
memblock.memory. So, without this change, fill_subsection_map will return -EEXIST.

I would say it's not a good idea to insert no-map memblock into subsection_map, and I have no idea why sparse do this.
So, I simply skip no-map region here.

As for safety:
1. The caller of fill_subsection_map are mhp and *_memremap_pages functions, no-map regions are not related to them, so existing codes won't be broken.
2. This change doesn't change memblock and subsection_map.

 > 
 > If the subsection map already spans memory (iow, subsection map is set)
 > you intend to add, then something already added memory in that range?

No matter with or without this patch, fill_subsection_map will always return -EEXIST for mapped memory, so that's not a problem.
The key point here is I allowed no-map region to pass the check and didn't change anything to the mapped region.

Regards,
Li