Skip to content

Commit

Permalink
powerpc/64s: Fix THP PMD collapse serialisation
Browse files Browse the repository at this point in the history
commit 33258a1 upstream.

Commit 1b2443a ("powerpc/book3s64: Avoid multiple endian
conversion in pte helpers") changed the actual bitwise tests in
pte_access_permitted by using pte_write() and pte_present() helpers
rather than raw bitwise testing _PAGE_WRITE and _PAGE_PRESENT bits.

The pte_present() change now returns true for PTEs which are
!_PAGE_PRESENT and _PAGE_INVALID, which is the combination used by
pmdp_invalidate() to synchronize access from lock-free lookups.
pte_access_permitted() is used by pmd_access_permitted(), so allowing
GUP lock free access to proceed with such PTEs breaks this
synchronisation.

This bug has been observed on a host using the hash page table MMU,
with random crashes and corruption in guests, usually together with
bad PMD messages in the host.

Fix this by adding an explicit check in pmd_access_permitted(), and
documenting the condition explicitly.

The pte_write() change should be okay, and would prevent GUP from
falling back to the slow path when encountering savedwrite PTEs, which
matches what x86 (that does not implement savedwrite) does.

Fixes: 1b2443a ("powerpc/book3s64: Avoid multiple endian conversion in pte helpers")
Cc: stable@vger.kernel.org # v4.20+
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
  • Loading branch information
Nicholas Piggin authored and Greg Kroah-Hartman committed Jun 19, 2019
1 parent c3871b7 commit ee6688c
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 0 deletions.
30 changes: 30 additions & 0 deletions arch/powerpc/include/asm/book3s/64/pgtable.h
Original file line number Diff line number Diff line change
Expand Up @@ -875,6 +875,23 @@ static inline int pmd_present(pmd_t pmd)
return false;
}

static inline int pmd_is_serializing(pmd_t pmd)
{
/*
* If the pmd is undergoing a split, the _PAGE_PRESENT bit is clear
* and _PAGE_INVALID is set (see pmd_present, pmdp_invalidate).
*
* This condition may also occur when flushing a pmd while flushing
* it (see ptep_modify_prot_start), so callers must ensure this
* case is fine as well.
*/
if ((pmd_raw(pmd) & cpu_to_be64(_PAGE_PRESENT | _PAGE_INVALID)) ==
cpu_to_be64(_PAGE_INVALID))
return true;

return false;
}

static inline int pmd_bad(pmd_t pmd)
{
if (radix_enabled())
Expand Down Expand Up @@ -1090,6 +1107,19 @@ static inline int pmd_protnone(pmd_t pmd)
#define pmd_access_permitted pmd_access_permitted
static inline bool pmd_access_permitted(pmd_t pmd, bool write)
{
/*
* pmdp_invalidate sets this combination (which is not caught by
* !pte_present() check in pte_access_permitted), to prevent
* lock-free lookups, as part of the serialize_against_pte_lookup()
* synchronisation.
*
* This also catches the case where the PTE's hardware PRESENT bit is
* cleared while TLB is flushed, which is suboptimal but should not
* be frequent.
*/
if (pmd_is_serializing(pmd))
return false;

return pte_access_permitted(pmd_pte(pmd), write);
}

Expand Down
3 changes: 3 additions & 0 deletions arch/powerpc/mm/pgtable-book3s64.c
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,9 @@ pmd_t pmdp_invalidate(struct vm_area_struct *vma, unsigned long address,
/*
* This ensures that generic code that rely on IRQ disabling
* to prevent a parallel THP split work as expected.
*
* Marking the entry with _PAGE_INVALID && ~_PAGE_PRESENT requires
* a special case check in pmd_access_permitted.
*/
serialize_against_pte_lookup(vma->vm_mm);
return __pmd(old_pmd);
Expand Down

0 comments on commit ee6688c

Please sign in to comment.