linux-kernel - Re: [PATCH v2 1/3] mm/hugetlb: Fix race condition of uffd missing/minor handling

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, 4 Oct 2022 09:49:37 -0400
From:   Peter Xu <peterx@...hat.com>
To:     David Hildenbrand <david@...hat.com>
Cc:     linux-kernel@...r.kernel.org, linux-mm@...ck.org,
        Andrew Morton <akpm@...ux-foundation.org>,
        Nadav Amit <nadav.amit@...il.com>,
        Mike Kravetz <mike.kravetz@...cle.com>,
        Andrea Arcangeli <aarcange@...hat.com>,
        Axel Rasmussen <axelrasmussen@...gle.com>,
        Mike Rapoport <rppt@...ux.vnet.ibm.com>
Subject: Re: [PATCH v2 1/3] mm/hugetlb: Fix race condition of uffd
 missing/minor handling

On Tue, Oct 04, 2022 at 02:19:36PM +0200, David Hildenbrand wrote:
> That looks kind-of ugly now. I wonder if it would be worth factoring that
> handling out into a separate function and reusing it at two places. Would
> get rid of one level of code indent at least.
> 
> Apart from that, LGTM. Although the lockless reading of the PTE screams for
> more trouble in the future :)

Right there's potential to further rework it, I am just not sure whether
that could be common enough so that we can start to take pg lock for the
whole region (then we'll need to release for either page lock or alloc).
Not really sure whether that'll be worth the effort.

However, at least uffd minor doesn't really need the page lock so we can
optimize it with a find_get_page() earlier then the missing mode can be
moved over too (following a lock_page?).  Maybe I should give it a shot.

For this one I'll keep it simple since I think we should have it for stable
too.  Thanks for the review!

-- 
Peter Xu