linux-kernel - Re: [PATCH 1/3] mm/memory-failure.c: avoid false-postive PageSwapCache test

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]

Date:   Tue, 12 Apr 2022 06:37:25 +0000
From:   HORIGUCHI NAOYA(堀口　直也) 
        <naoya.horiguchi@....com>
To:     Miaohe Lin <linmiaohe@...wei.com>
CC:     "akpm@...ux-foundation.org" <akpm@...ux-foundation.org>,
        "shy828301@...il.com" <shy828301@...il.com>,
        "mike.kravetz@...cle.com" <mike.kravetz@...cle.com>,
        "david@...hat.com" <david@...hat.com>,
        "linux-mm@...ck.org" <linux-mm@...ck.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/3] mm/memory-failure.c: avoid false-postive
 PageSwapCache test

On Mon, Apr 11, 2022 at 09:19:26PM +0800, Miaohe Lin wrote:
> On 2022/4/11 14:35, HORIGUCHI NAOYA(堀口 直也) wrote:
> > On Thu, Apr 07, 2022 at 09:03:50PM +0800, Miaohe Lin wrote:
> >> PageSwapCache is only reliable when PageAnon is true because PG_swapcache
> >> serves as PG_owner_priv_1 which can be used by fs if it's pagecache page.
> >> So we should test PageAnon to distinguish pagecache page from swapcache
> >> page to avoid false-postive PageSwapCache test.
> >>
> >> Signed-off-by: Miaohe Lin <linmiaohe@...wei.com>
> >> ---
> >>  mm/memory-failure.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
> >> index ef402b490663..2e97302d62e4 100644
> >> --- a/mm/memory-failure.c
> >> +++ b/mm/memory-failure.c
> >> @@ -2262,7 +2262,7 @@ static int __soft_offline_page(struct page *page)
> >>  		return 0;
> >>  	}
> >>  
> >> -	if (!PageHuge(page) && PageLRU(page) && !PageSwapCache(page))
> >> +	if (!PageHuge(page) && PageLRU(page) && !PageAnon(page))
> >>  		/*
> >>  		 * Try to invalidate first. This should work for
> >>  		 * non dirty unmapped page cache pages.
> >> -- 
> > 
> > I foudn that with this change the following VM_BUG_ON_FOLIO() is triggered
> > when calling soft-offline for a swapcache.  Maybe we need check both of
> > PageAnon and PageSwapCache instead of either?
> > 
> 
> Many thanks for your test! This is my overlook. Sorry about it! :( The root cause is that the page is
> added into swapcache and lru( so that it can pass the HWPoisonHandlable check) but page anon is not
> set yet due to page lock is held by __soft_offline_page. So we have the below core dump:
> 
> [   41.232172] page:0000000033d8a20c refcount:0 mapcount:0 mapping:00000000bc103d88 index:0x36d pfn:0x14359b
> 										^^^ page is not anon
> 
> [   41.236576] flags: 0x57ffffc0080415(locked|uptodate|lru|owner_priv_1|swapbacked|node=1|zone=2|lastcpupid=0x1fffff)
> 								^^^^^^^^^^^^^^^^^^ page is in swapcache
> 
> It seems we can check !PageAnon(page) && !PageSwapCache(page), as you suggested, to fix this issue. But maybe I
> should drop this patch because invalidate_inode_page will always return 0 for PageAnon due to folio_mapping == NULL.
> So nothing is really done for anonymous page here. And the origin !PageSwapCache(page) check should do the right work.

Thanks for clarification.

> Or we shouldn't even try to call invalidate_inode_page with anonymous page in principle?

I think just keeping the current behavior is fine (because as you stated
above invalidate_inode_page() simple ignores anonymous pages).

Thanks,
Naoya Horiguchi

> BTW: PageSwapCache should be reliable here as folio_test_swapbacked is checked implicitly inside it. In such case, PG_swapcache
> can't serve as PG_owner_priv_1 as pagecache page shouldn't set PG_swapbacked (shmem will set PG_swapbacked but PG_owner_priv_1
> is not used anyway). Or am I miss something again?