linux-kernel - Re: [PATCH] mm/migrate: fix race between lock page and clear PG

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: <20220321162613.0a3510d18f1fa0f1637e9661@linux-foundation.org>
Date:   Mon, 21 Mar 2022 16:26:13 -0700
From:   Andrew Morton <akpm@...ux-foundation.org>
To:     Matthew Wilcox <willy@...radead.org>
Cc:     Andrew Yang <andrew.yang@...iatek.com>,
        Matthias Brugger <matthias.bgg@...il.com>,
        Vlastimil Babka <vbabka@...e.cz>,
        David Howells <dhowells@...hat.com>,
        William Kucharski <william.kucharski@...cle.com>,
        David Hildenbrand <david@...hat.com>,
        Yang Shi <shy828301@...il.com>, Marc Zyngier <maz@...nel.org>,
        linux-kernel@...r.kernel.org, linux-mm@...ck.org,
        linux-arm-kernel@...ts.infradead.org,
        linux-mediatek@...ts.infradead.org, wsd_upstream@...iatek.com,
        Nicholas Tang <nicholas.tang@...iatek.com>,
        Kuan-Ying Lee <Kuan-Ying.Lee@...iatek.com>
Subject: Re: [PATCH] mm/migrate: fix race between lock page and clear
 PG_Isolated

On Tue, 15 Mar 2022 03:32:16 +0000 Matthew Wilcox <willy@...radead.org> wrote:

> On Tue, Mar 15, 2022 at 11:05:15AM +0800, Andrew Yang wrote:
> > When memory is tight, system may start to compact memory for large
> > continuous memory demands. If one process tries to lock a memory page
> > that is being locked and isolated for compaction, it may wait a long time
> > or even forever. This is because compaction will perform non-atomic
> > PG_Isolated clear while holding page lock, this may overwrite PG_waiters
> > set by the process that can't obtain the page lock and add itself to the
> > waiting queue to wait for the lock to be unlocked.
> > 
> > CPU1                            CPU2
> > lock_page(page); (successful)
> >                                 lock_page(); (failed)
> > __ClearPageIsolated(page);      SetPageWaiters(page) (may be overwritten)
> > unlock_page(page);
> > 
> > The solution is to not perform non-atomic operation on page flags while
> > holding page lock.
> > 
> > Signed-off-by: andrew.yang <andrew.yang@...iatek.com>
> > ---
> >  include/linux/page-flags.h |  2 +-
> >  mm/migrate.c               | 12 ++++++------
> >  2 files changed, 7 insertions(+), 7 deletions(-)
> > 
> > diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
> > index 1c3b6e5c8bfd..64a84a9835cb 100644
> > --- a/include/linux/page-flags.h
> > +++ b/include/linux/page-flags.h
> > @@ -918,7 +918,7 @@ PAGE_TYPE_OPS(Guard, guard)
> >  
> >  extern bool is_free_buddy_page(struct page *page);
> >  
> > -__PAGEFLAG(Isolated, isolated, PF_ANY);
> > +PAGEFLAG(Isolated, isolated, PF_ANY);
> 
> Agreed.  Further, page cannot be a tail page (this is implied by the
> get_page_unless_zero() as tailpages have a zero refcount, and it
> is assumed by __PageMovable() as page->mapping is undefined for tail
> pages).  So this can actually be:
> 
> +PAGEFLAG(Isolated, isolated, PF_NO_TAIL);
> 
> I considered PF_ONLY_HEAD, but there are a lot more places that _check_
> PageIsolated() and I don't want to prove that they're all definitely
> working on head pages.

There was no followup to this.  I'll send the patch upstream as-is, but
please let's not forget this idea.