linux-kernel - Re: [PATCH v4] mm/hwpoison: fix race between hugetlb free/demotion and memory_failure

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]

Message-ID: <20220317144443.GB32934@hori.linux.bs1.fc.nec.co.jp>
Date:   Thu, 17 Mar 2022 14:44:43 +0000
From:   HORIGUCHI NAOYA(堀口　直也) 
        <naoya.horiguchi@....com>
To:     Miaohe Lin <linmiaohe@...wei.com>
CC:     Mike Kravetz <mike.kravetz@...cle.com>,
        Naoya Horiguchi <naoya.horiguchi@...ux.dev>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Yang Shi <shy828301@...il.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Linux-MM <linux-mm@...ck.org>
Subject: Re: [PATCH v4] mm/hwpoison: fix race between hugetlb free/demotion
 and memory_failure_hugetlb()

On Thu, Mar 17, 2022 at 05:28:13PM +0800, Miaohe Lin wrote:
> On 2022/3/17 6:51, Mike Kravetz wrote:
> > On 3/16/22 05:07, Naoya Horiguchi wrote:
> >> From: Miaohe Lin <linmiaohe@...wei.com>
...
> > 
> > In my testing, there is a change in behavior that may not be intended.
> > 
> > My test strategy is:
> > - allocate two hugetlb pages
> > - create a mapping which reserves those two pages, but does not fault them in
> >   - as a result, the pages are on the free list but can not be freed
> > - inject error on a subpage of one of the huge pages
> >   - echo 0xYYY > /sys/kernel/debug/hwpoison/corrupt-pfn
> > - memory error code will call dissolve_free_huge_page
> >   - dissolve_free_huge_page returns -EBUSY because
> >     h->free_huge_pages - h->resv_huge_pages == 0
> > - We never end up setting Poison on the page with error or head page
> > - Huge page sitting on free list with error in subpage and not marked
> > - huge page with error could be given to an application or returned to buddy
> > 
> > Prior to this change, Poison would be set on the head page
> > 
> 
> Many thanks for pointing this out. IIUC, this change in behavior should be a bit
> unintended. We're trying to avoid setting PageHWPoison flag on the wrong page so
> we have to set the PageHWPoison flag after passing prechecks as commit log said.
> But there is room for improvement, e.g. when page changed to single page or another
> compound-size page after we grab the page refcnt, we could also set PageHWPoison
> before bailing out ? There might be something more we can do?

Yha, we could have more improvement around it.  I think that we can add
SetPageHWPoison near action_result(MF_MSG_DIFFERENT_PAGE_SIZE), but on which
page?  Maybe setting PageHWPoison on the raw page (not head page of new
compound page) is better because the new compound pages should not be hugetlb.
What about action_result(MF_MSG_UNKNOWN)?  Maybe we should do the same.
IOW, setting PageHWPoison on the head page is justified only when the
page is surely a hugepage.

Thanks,
Naoya Horiguchi