Skip to content
Snippets Groups Projects
  • Ryan Roberts's avatar
    d02591d2
    mm: fix race between __split_huge_pmd_locked() and GUP-fast · d02591d2
    Ryan Roberts authored and Frieder Schrempf's avatar Frieder Schrempf committed
    commit 3a5a8d34 upstream.
    
    __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: 84c3fc4e ("mm: thp: check pmd migration entry in common path")
    Signed-off-by: default avatarRyan Roberts <ryan.roberts@arm.com>
    Reviewed-by: default avatarZi Yan <ziy@nvidia.com>
    Reviewed-by: default avatarAnshuman Khandual <anshuman.khandual@arm.com>
    Acked-by: default avatarDavid 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: default avatarAndrew Morton <akpm@linux-foundation.org>
    Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
    d02591d2
    History
    mm: fix race between __split_huge_pmd_locked() and GUP-fast
    Ryan Roberts authored and Frieder Schrempf's avatar Frieder Schrempf committed
    commit 3a5a8d34 upstream.
    
    __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: 84c3fc4e ("mm: thp: check pmd migration entry in common path")
    Signed-off-by: default avatarRyan Roberts <ryan.roberts@arm.com>
    Reviewed-by: default avatarZi Yan <ziy@nvidia.com>
    Reviewed-by: default avatarAnshuman Khandual <anshuman.khandual@arm.com>
    Acked-by: default avatarDavid 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: default avatarAndrew Morton <akpm@linux-foundation.org>
    Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>