Re: [PATCH 4.19.y] mm: fix race between __split_huge_pmd_locked() and GUP-fast

[Greg KH <gregkh@linuxfoundation.org>]

On Mon, Jun 24, 2024 at 10:27:43AM +0100, Ryan Roberts wrote:
> __split_huge_pmd_locked() can be called for a present THP, devmap or
> (non-present) migration entry.  It calls pmdp_invalidate() unconditionally
> on the pmdp and only determines if it is present or not based on the
> returned old pmd.  This is a problem for the migration entry case because
> pmd_mkinvalid(), called by pmdp_invalidate() must only be called for a
> present pmd.
> 
> On arm64 at least, pmd_mkinvalid() will mark the pmd such that any future
> call to pmd_present() will return true.  And therefore any lockless
> pgtable walker could see the migration entry pmd in this state and start
> interpretting the fields as if it were present, leading to BadThings (TM).
> GUP-fast appears to be one such lockless pgtable walker.
> 
> x86 does not suffer the above problem, but instead pmd_mkinvalid() will
> corrupt the offset field of the swap entry within the swap pte.  See link
> below for discussion of that problem.
> 
> Fix all of this by only calling pmdp_invalidate() for a present pmd.  And
> for good measure let's add a warning to all implementations of
> pmdp_invalidate[_ad]().  I've manually reviewed all other
> pmdp_invalidate[_ad]() call sites and believe all others to be conformant.
> 
> This is a theoretical bug found during code review.  I don't have any test
> case to trigger it in practice.
> 
> Link: https://lkml.kernel.org/r/20240501143310.1381675-1-ryan.roberts@arm.com
> Link: https://lore.kernel.org/all/0dd7827a-6334-439a-8fd0-43c98e6af22b@arm.com/
> Fixes: 84c3fc4e9c56 ("mm: thp: check pmd migration entry in common path")
> Signed-off-by: Ryan Roberts <ryan.roberts@arm.com>
> Reviewed-by: Zi Yan <ziy@nvidia.com>
> Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>
> Acked-by: David Hildenbrand <david@redhat.com>
> Cc: Andreas Larsson <andreas@gaisler.com>
> Cc: Andy Lutomirski <luto@kernel.org>
> Cc: Aneesh Kumar K.V <aneesh.kumar@kernel.org>
> Cc: Borislav Petkov (AMD) <bp@alien8.de>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Christian Borntraeger <borntraeger@linux.ibm.com>
> Cc: Christophe Leroy <christophe.leroy@csgroup.eu>
> Cc: Dave Hansen <dave.hansen@linux.intel.com>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Naveen N. Rao <naveen.n.rao@linux.ibm.com>
> Cc: Nicholas Piggin <npiggin@gmail.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Sven Schnelle <svens@linux.ibm.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Will Deacon <will@kernel.org>
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> (cherry picked from commit 3a5a8d343e1cf96eb9971b17cbd4b832ab19b8e7)
> Signed-off-by: Ryan Roberts <ryan.roberts@arm.com>

All now queued up, thanks.

greg k-h