linux-kernel - Re: [PATCH 1/4] mm/memory-failure.c: fix race with changing page compound again

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]

Message-ID: <79f79813-bb9a-bfff-0837-a24d14da0b19@oracle.com>
Date:   Fri, 4 Mar 2022 11:32:02 -0800
From:   Mike Kravetz <mike.kravetz@...cle.com>
To:     HORIGUCHI NAOYA(堀口 直也) 
        <naoya.horiguchi@....com>, Miaohe Lin <linmiaohe@...wei.com>
Cc:     "akpm@...ux-foundation.org" <akpm@...ux-foundation.org>,
        "linux-mm@...ck.org" <linux-mm@...ck.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/4] mm/memory-failure.c: fix race with changing page
 compound again

On 3/4/22 00:26, HORIGUCHI NAOYA(堀口 直也) wrote:
> On Mon, Feb 28, 2022 at 10:02:42PM +0800, Miaohe Lin wrote:
>> There is a race window where we got the compound_head, the hugetlb page
>> could be freed to buddy, or even changed to another compound page just
>> before we try to get hwpoison page. If this happens, just bail out.
> 
> I think that when some hugetlb page is about to change into other type/size
> of compound page, it has to go through buddy allocator because hugetlb pages
> are maintained in separate memory allocator and they never change into other
> normal state directly.  memory_failure_hugetlb() takes refcount before
> lock_page(), so the hugetlb page seems not change between get_hwpoison_page()
> and lock_page(). So it this new check really necessary?

A hugetlb page could change size without going through buddy via the new
demote functionality [1].  Only hugetlb pages on the hugetlb free list can
be demoted.  

We should not demote a page if poison is set.  However, there is no check in
the demote code.  IIUC, poison is set early in the memory error handling
process, even before taking ref on page.  Demote code needs to be fixed so
that poisoned pages are not demoted.  I can do that.

With this change in place, then I think Naoya's statement that hugetlb pages
can not change state is correct and this patch is not necessary.

Does that sound reasonable?

[1] https://lore.kernel.org/linux-mm/20211007181918.136982-1-mike.kravetz@oracle.com/
-- 
Mike Kravetz