diff mbox series

powerpc/64s/radix: Fix missing ptesync in flush_cache_vmap

Message ID 20180606014008.20363-1-npiggin@gmail.com (mailing list archive)
State Accepted
Commit ff5bc793e47b537bf3e904fada585e102c54dd8b
Headers show
Series powerpc/64s/radix: Fix missing ptesync in flush_cache_vmap | expand

Commit Message

Nicholas Piggin June 6, 2018, 1:40 a.m. UTC
There is a typo in f1cb8f9beb ("powerpc/64s/radix: avoid ptesync after
set_pte and ptep_set_access_flags") config ifdef, which results in the
necessary ptesync not being issued after vmalloc.

This causes random kernel faults in module load, bpf load, anywhere
that vmalloc mappings are used.

After correcting the code, this survives a guest kernel booting
hundreds of times where previously there would be a crash every few
boots (I haven't noticed the crash on host, perhaps due to different
TLB and page table walking behaviour in hardware).

A memory clobber is also added to the flush, just to be sure it won't
be reordered with the pte set or the subsequent mapping access.

Fixes: f1cb8f9beb ("powerpc/64s/radix: avoid ptesync after set_pte and ptep_set_access_flags")
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
This code obviously can't have been tested properly, I'm sorry about
that. I tested with the ptesync and then "tidied up" the patch by
putting it in ifdef, and didn't test the final patch sufficiently :(

 arch/powerpc/include/asm/cacheflush.h | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

Comments

Michael Ellerman June 7, 2018, 12:17 a.m. UTC | #1
On Wed, 2018-06-06 at 01:40:08 UTC, Nicholas Piggin wrote:
> There is a typo in f1cb8f9beb ("powerpc/64s/radix: avoid ptesync after
> set_pte and ptep_set_access_flags") config ifdef, which results in the
> necessary ptesync not being issued after vmalloc.
> 
> This causes random kernel faults in module load, bpf load, anywhere
> that vmalloc mappings are used.
> 
> After correcting the code, this survives a guest kernel booting
> hundreds of times where previously there would be a crash every few
> boots (I haven't noticed the crash on host, perhaps due to different
> TLB and page table walking behaviour in hardware).
> 
> A memory clobber is also added to the flush, just to be sure it won't
> be reordered with the pte set or the subsequent mapping access.
> 
> Fixes: f1cb8f9beb ("powerpc/64s/radix: avoid ptesync after set_pte and ptep_set_access_flags")
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/ff5bc793e47b537bf3e904fada585e

cheers
diff mbox series

Patch

diff --git a/arch/powerpc/include/asm/cacheflush.h b/arch/powerpc/include/asm/cacheflush.h
index e9662648e72d..0d72ec75da63 100644
--- a/arch/powerpc/include/asm/cacheflush.h
+++ b/arch/powerpc/include/asm/cacheflush.h
@@ -23,10 +23,9 @@ 
 #define flush_cache_range(vma, start, end)	do { } while (0)
 #define flush_cache_page(vma, vmaddr, pfn)	do { } while (0)
 #define flush_icache_page(vma, page)		do { } while (0)
-#define flush_cache_vmap(start, end)		do { } while (0)
 #define flush_cache_vunmap(start, end)		do { } while (0)
 
-#ifdef CONFIG_BOOK3S_64
+#ifdef CONFIG_PPC_BOOK3S_64
 /*
  * Book3s has no ptesync after setting a pte, so without this ptesync it's
  * possible for a kernel virtual mapping access to return a spurious fault
@@ -34,7 +33,7 @@ 
  * not expect this type of fault. flush_cache_vmap is not exactly the right
  * place to put this, but it seems to work well enough.
  */
-#define flush_cache_vmap(start, end)		do { asm volatile("ptesync"); } while (0)
+#define flush_cache_vmap(start, end)		do { asm volatile("ptesync" ::: "memory"); } while (0)
 #else
 #define flush_cache_vmap(start, end)		do { } while (0)
 #endif