powerpc/mm: Fix bugs in huge page hashing

There's a couple of nasty bugs lurking in our huge page hashing code.

First, we don't check the access permission atomically with setting
the _PAGE_BUSY bit, which means that the PTE value we end up using
for the hashing might be different than the one we have checked
the access permissions for.

We've seen cases where that leads us to try to use an invalidated
PTE for hashing, causing all sort of "interesting" issues.

Then, we also failed to set _PAGE_DIRTY on a write access.

Finally, a minor tweak but we should return 0 when we find the
PTE busy, in order to just re-execute the access, rather than 1
which means going to do_page_fault().
Signed-off-by: default avatarBenjamin Herrenschmidt <benh@kernel.crashing.org>
---
parent ca91e6c0
...@@ -21,21 +21,13 @@ int __hash_page_huge(unsigned long ea, unsigned long access, unsigned long vsid, ...@@ -21,21 +21,13 @@ int __hash_page_huge(unsigned long ea, unsigned long access, unsigned long vsid,
unsigned long old_pte, new_pte; unsigned long old_pte, new_pte;
unsigned long va, rflags, pa, sz; unsigned long va, rflags, pa, sz;
long slot; long slot;
int err = 1;
BUG_ON(shift != mmu_psize_defs[mmu_psize].shift); BUG_ON(shift != mmu_psize_defs[mmu_psize].shift);
/* Search the Linux page table for a match with va */ /* Search the Linux page table for a match with va */
va = hpt_va(ea, vsid, ssize); va = hpt_va(ea, vsid, ssize);
/* /* At this point, we have a pte (old_pte) which can be used to build
* Check the user's access rights to the page. If access should be
* prevented then send the problem up to do_page_fault.
*/
if (unlikely(access & ~pte_val(*ptep)))
goto out;
/*
* At this point, we have a pte (old_pte) which can be used to build
* or update an HPTE. There are 2 cases: * or update an HPTE. There are 2 cases:
* *
* 1. There is a valid (present) pte with no associated HPTE (this is * 1. There is a valid (present) pte with no associated HPTE (this is
...@@ -49,9 +41,17 @@ int __hash_page_huge(unsigned long ea, unsigned long access, unsigned long vsid, ...@@ -49,9 +41,17 @@ int __hash_page_huge(unsigned long ea, unsigned long access, unsigned long vsid,
do { do {
old_pte = pte_val(*ptep); old_pte = pte_val(*ptep);
if (old_pte & _PAGE_BUSY) /* If PTE busy, retry the access */
goto out; if (unlikely(old_pte & _PAGE_BUSY))
return 0;
/* If PTE permissions don't match, take page fault */
if (unlikely(access & ~old_pte))
return 1;
/* Try to lock the PTE, add ACCESSED and DIRTY if it was
* a write access */
new_pte = old_pte | _PAGE_BUSY | _PAGE_ACCESSED; new_pte = old_pte | _PAGE_BUSY | _PAGE_ACCESSED;
if (access & _PAGE_RW)
new_pte |= _PAGE_DIRTY;
} while(old_pte != __cmpxchg_u64((unsigned long *)ptep, } while(old_pte != __cmpxchg_u64((unsigned long *)ptep,
old_pte, new_pte)); old_pte, new_pte));
...@@ -127,8 +127,7 @@ int __hash_page_huge(unsigned long ea, unsigned long access, unsigned long vsid, ...@@ -127,8 +127,7 @@ int __hash_page_huge(unsigned long ea, unsigned long access, unsigned long vsid,
*/ */
if (unlikely(slot == -2)) { if (unlikely(slot == -2)) {
*ptep = __pte(old_pte); *ptep = __pte(old_pte);
err = -1; return -1;
goto out;
} }
new_pte |= (slot << 12) & (_PAGE_F_SECOND | _PAGE_F_GIX); new_pte |= (slot << 12) & (_PAGE_F_SECOND | _PAGE_F_GIX);
...@@ -138,9 +137,5 @@ int __hash_page_huge(unsigned long ea, unsigned long access, unsigned long vsid, ...@@ -138,9 +137,5 @@ int __hash_page_huge(unsigned long ea, unsigned long access, unsigned long vsid,
* No need to use ldarx/stdcx here * No need to use ldarx/stdcx here
*/ */
*ptep = __pte(new_pte & ~_PAGE_BUSY); *ptep = __pte(new_pte & ~_PAGE_BUSY);
return 0;
err = 0;
out:
return err;
} }
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment