Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
MIME-Version: 1.0
References: <20220331065640.5777-1-songmuchun@bytedance.com>
 <20220331065640.5777-2-songmuchun@bytedance.com> <d6e2581e-a8ac-4848-2c64-4a221bd03bca@arm.com>
In-Reply-To: <d6e2581e-a8ac-4848-2c64-4a221bd03bca@arm.com>
From:   Muchun Song <songmuchun@bytedance.com>
Date:   Tue, 5 Apr 2022 16:38:20 +0800
Message-ID: <CAMZfGtWyXmPwZWsH_pP_M7p30uBww8BdP0DRXQRjBkT_VP=uUA@mail.gmail.com>
Subject: Re: [PATCH v4 2/2] arm64: mm: hugetlb: Enable HUGETLB_PAGE_FREE_VMEMMAP
 for arm64
To:     Anshuman Khandual <anshuman.khandual@arm.com>
Cc:     Will Deacon <will@kernel.org>,
        Andrew Morton <akpm@linux-foundation.org>,
        David Hildenbrand <david@redhat.com>,
        Oscar Salvador <osalvador@suse.de>,
        Mike Kravetz <mike.kravetz@oracle.com>,
        David Rientjes <rientjes@google.com>,
        Mark Rutland <mark.rutland@arm.com>,
        Catalin Marinas <catalin.marinas@arm.com>, james.morse@arm.com,
        Barry Song <21cnbao@gmail.com>,
        LAK <linux-arm-kernel@lists.infradead.org>,
        LKML <linux-kernel@vger.kernel.org>,
        Linux Memory Management List <linux-mm@kvack.org>,
        Xiongchun duan <duanxiongchun@bytedance.com>,
        Fam Zheng <fam.zheng@bytedance.com>,
        Muchun Song <smuchun@gmail.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Tue, Apr 5, 2022 at 12:44 PM Anshuman Khandual
<anshuman.khandual@arm.com> wrote:
>
>
>
> On 3/31/22 12:26, Muchun Song wrote:
> > 1st concern:
> > '''
> > But what happens when a hot remove section's vmemmap area (which is
> > being teared down) is nearby another vmemmap area which is either created
> > or being destroyed for HugeTLB alloc/free purpose. As you mentioned
> > HugeTLB pages inside the hot remove section might be safe. But what about
> > other HugeTLB areas whose vmemmap area shares page table entries with
> > vmemmap entries for a section being hot removed ? Massive HugeTLB alloc
> > /use/free test cycle using memory just adjacent to a memory hotplug area,
> > which is always added and removed periodically, should be able to expose
> > this problem.
> > '''
> >
> > Answer: At the time memory is removed, all HugeTLB pages either have been
> > migrated away or dissolved.  So there is no race between memory hot remove
> > and free_huge_page_vmemmap().  Therefore, HugeTLB pages inside the hot
> > remove section is safe.  Let's talk your question "what about other
>
> HugeTLB pages inside the memory range is safe but concern is about the
> vmemmap mapping for the HugeTLB which might share intermediate entries
> with vmemmap mapping for the memory range/section being removed.

The shared page table level only could be PMD, PUD and PGD, the PTE
page table cannot be shared with other sections, and we only exchange
PTEs for vmemmap mapping.

>
> > HugeTLB areas whose vmemmap area shares page table entries with vmemmap
> > entries for a section being hot removed ?", the question is not
>
> Right.
>
> > established.  The minimal granularity size of hotplug memory 128MB (on
> > arm64, 4k base page), any HugeTLB smaller than 128MB is within a section,
> > then, there is no share PTE page tables between HugeTLB in this section
>
> 128MB is the hot removable granularity but, its corresponding vmemmap
> range is smaller i.e (128MB/4K) * sizeof(struct page). Memory section
> getting hot removed (its vmemmap mapping being teared down) along with
> HugeTLB (on another section) vmemmap remap operation, could not collide
> while inside vmemmap mapping areas on init_mm ?

The boundary address of a section is aligned with 128MB and its
corresponding vmemmap boundary address is aligned with 2MB
which is mapped with a separated PTE page table (or a PMD entry).
Different sections do not share the same PTE, there are no conflicts
between a hot removed section and a remapping vmemmap section
since we are operating on different PTE. Right?

>
> > and ones in other sections and a HugeTLB page could not cross two
> > sections.  In this case, the section cannot be freed.  Any HugeTLB bigger
>
> Right, they dont cross into two different sections.
>
> > than 128MB (section size) whose vmemmap pages is an integer multiple of
> > 2MB (PMD-mapped).  As long as:
> >
> >   1) HugeTLBs are naturally aligned, power-of-two sizes
> >   2) The HugeTLB size >= the section size
> >   3) The HugeTLB size >= the vmemmap leaf mapping size
> >
> > Then a HugeTLB will not share any leaf page table entries with *anything
> > else*, but will share intermediate entries.  In this case, at the time memory
> > is removed, all HugeTLB pages either have been migrated away or dissolved.
> > So there is also no race between memory hot remove and
> > free_huge_page_vmemmap().
>
> If they just share intermediate entries, free_empty_tables() will not free
> up page table pages, as there will be valid non-zero entries in them. But

Right.

> the problem here is not UAF, its accessing wrong entries and crashing while
> de-referncing the pointer. Hence I am wondering if no such scenario can be
> possible.
>

What's the wrong entries? You mean the reused vmemmap page entries?
If so, I think free_empty_tables() will not cause any crash.  The hot removed
operation couldn't reach those entries since those addresses mapped with
those reused entries are not included in the range of the hot removed section.

Thanks.