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@xxxxxxx > Link: https://lore.kernel.org/all/0dd7827a-6334-439a-8fd0-43c98e6af22b@xxxxxxx/ > Fixes: 84c3fc4e9c56 ("mm: thp: check pmd migration entry in common path") > Signed-off-by: Ryan Roberts <ryan.roberts@xxxxxxx> > Reviewed-by: Zi Yan <ziy@xxxxxxxxxx> > Reviewed-by: Anshuman Khandual <anshuman.khandual@xxxxxxx> > Acked-by: David Hildenbrand <david@xxxxxxxxxx> > Cc: Andreas Larsson <andreas@xxxxxxxxxxx> > Cc: Andy Lutomirski <luto@xxxxxxxxxx> > Cc: Aneesh Kumar K.V <aneesh.kumar@xxxxxxxxxx> > Cc: Borislav Petkov (AMD) <bp@xxxxxxxxx> > Cc: Catalin Marinas <catalin.marinas@xxxxxxx> > Cc: Christian Borntraeger <borntraeger@xxxxxxxxxxxxx> > Cc: Christophe Leroy <christophe.leroy@xxxxxxxxxx> > Cc: Dave Hansen <dave.hansen@xxxxxxxxxxxxxxx> > Cc: "David S. Miller" <davem@xxxxxxxxxxxxx> > Cc: Ingo Molnar <mingo@xxxxxxxxxx> > Cc: Jonathan Corbet <corbet@xxxxxxx> > Cc: Mark Rutland <mark.rutland@xxxxxxx> > Cc: Naveen N. Rao <naveen.n.rao@xxxxxxxxxxxxx> > Cc: Nicholas Piggin <npiggin@xxxxxxxxx> > Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx> > Cc: Sven Schnelle <svens@xxxxxxxxxxxxx> > Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx> > Cc: Will Deacon <will@xxxxxxxxxx> > Cc: <stable@xxxxxxxxxxxxxxx> > Signed-off-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> > (cherry picked from commit 3a5a8d343e1cf96eb9971b17cbd4b832ab19b8e7) > Signed-off-by: Ryan Roberts <ryan.roberts@xxxxxxx> All now queued up, thanks. greg k-h
