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: <Yzw58fSsSmxSqfYo@x1n>
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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ