linux-kernel - Re: [PATCH 2/2] mm/khugepaged: skip shmem with userfaultfd

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:   Thu, 16 Feb 2023 18:07:25 -0500
From:   Peter Xu <peterx@...hat.com>
To:     Yang Shi <shy828301@...il.com>
Cc:     David Stevens <stevensd@...omium.org>, linux-mm@...ck.org,
        Matthew Wilcox <willy@...radead.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        "Kirill A . Shutemov" <kirill@...temov.name>,
        David Hildenbrand <david@...hat.com>,
        Hugh Dickins <hughd@...gle.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] mm/khugepaged: skip shmem with userfaultfd

Hi, Yang,

On Thu, Feb 16, 2023 at 01:58:55PM -0800, Yang Shi wrote:
> > IIUC we released it before copying the pages:
> 
> The huge page is locked until the copy is done. It should be fine
> unless the users inspect the page content without acquiring page lock.

The current patch from David has replaced "insert hpage into holes" with
"insert RETRY entries into holes", so IMHO the hpage is not visible at all
when releasing page cache lock here.

All the accessors (including RCU protected ones to access page cache; those
may not need to take the page lock) should be spinning on the RETRY entry,
which it seems fine to me.  But my question was whether it's legal to keep
them spinning even after releasing the page cache lock.

Thanks,

> 
> >
> > xa_locked:
> >         xas_unlock_irq(&xas);  <-------------------------------- here
> > xa_unlocked:
> >
> >         /*
> >          * If collapse is successful, flush must be done now before copying.
> >          * If collapse is unsuccessful, does flush actually need to be done?
> >          * Do it anyway, to clear the state.
> >          */
> >         try_to_unmap_flush();
> >
> > Before insertion of the multi-index:
> >
> >         /* Join all the small entries into a single multi-index entry. */
> >         xas_set_order(&xas, start, HPAGE_PMD_ORDER);
> >         xas_store(&xas, hpage);
> >
> > Thanks,
> >
> > --
> > Peter Xu
> >
> 

-- 
Peter Xu