linux-kernel - Re: [External] Re: [PATCH] mm: migrate: fix missing update page_private to hugetlb_page

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] [day] [month] [year] [list]

Date:   Fri, 21 May 2021 10:17:08 +0800
From:   Muchun Song <songmuchun@...edance.com>
To:     Mike Kravetz <mike.kravetz@...cle.com>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        Oscar Salvador <osalvador@...e.de>,
        Michal Hocko <mhocko@...e.com>,
        David Hildenbrand <david@...hat.com>,
        Matthew Wilcox <willy@...radead.org>,
        Linux Memory Management List <linux-mm@...ck.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Xiongchun duan <duanxiongchun@...edance.com>,
        zhengqi.arch@...edance.com, fam.zheng@...edance.com,
        Anshuman Khandual <anshuman.khandual@....com>
Subject: Re: [External] Re: [PATCH] mm: migrate: fix missing update
 page_private to hugetlb_page_subpool

On Fri, May 21, 2021 at 2:17 AM Mike Kravetz <mike.kravetz@...cle.com> wrote:
>
> On 5/19/21 10:13 PM, Muchun Song wrote:
> > On Thu, May 20, 2021 at 12:24 PM Andrew Morton
> > <akpm@...ux-foundation.org> wrote:
> >>
> >> On Thu, 20 May 2021 10:59:49 +0800 Muchun Song <songmuchun@...edance.com> wrote:
> >>
> >>> Since commit d6995da31122 ("hugetlb: use page.private for hugetlb specific
> >>> page flags") converts page.private for hugetlb specific page flags. We
> >>> should use hugetlb_page_subpool() to get the subpool pointer instead of
> >>> page_private(). The commit forgot to update it in the page migration
> >>> routine. So fix it.
> >>>
> >>> ...
> >>>
> >>> --- a/mm/migrate.c
> >>> +++ b/mm/migrate.c
> >>> @@ -1290,7 +1290,7 @@ static int unmap_and_move_huge_page(new_page_t get_new_page,
> >>>        * page_mapping() set, hugetlbfs specific move page routine will not
> >>>        * be called and we could leak usage counts for subpools.
> >>>        */
> >>> -     if (page_private(hpage) && !page_mapping(hpage)) {
> >>> +     if (hugetlb_page_subpool(hpage) && !page_mapping(hpage)) {
> >>>               rc = -EBUSY;
> >>>               goto out_unlock;
> >>>       }
> >>
> >> So it uses the wrong page*, so this isn't just a cosmetic fix.  One
> >> cannot tell from this changelog.
> >>
> >> Please describe the runtime effects of this bug.  Please always include
> >> this information when fixing bugs.  And when adding them.
> >
> > OK. I should update the commit log in the next version.
> >
>
> Unfortunately, it is more than a cosmetic fix.
>
> This 'could' prevent the migration of hugetlb pages.  page_private(hpage)
> is now used for hugetlb page specific flags.  At migration time, the
> only flag which could be set is HPageVmemmapOptimized.  This flag will
> only be set if the new vmemmap reduction feature is enabled.  In
> addition, !page_mapping() implies an anonymous mapping.  So, this will
> prevent migration of hugetb pages in anonymous mappings if the vmemmap
> reduction feature is enabled.
>
> In addition, that if statement checked for the rare race condition of a
> page being migrated while in the process of being freed.  Since that
> check is now wrong, we could leak hugetlb subpool usage counts.
>
> Muchun, feel free to use as much of that as you would like in the
> updated commit message.

Thanks Mike. I'll use this.

> --
> Mike Kravetz