MIPS: Fix HIGHMEM crash in __update_cache().

PATCH In review Lars Persson 3 participants 5 messages Apr 10, 2015 – Apr 14, 2015 MIPS

Share & tools

Lars Persson

author

2015-04-10 13:21 UTC #0 lore raw

PATCH 1/1 MIPS: Fix HIGHMEM crash in __update_cache().

Commit 8b5fe5e54b47 ("MIPS: Fix race condition in lazy cache flushing.") triggered NULL pointer dereferences on systems with HIGHMEM.

The problem was caused by not clearing the PG_dcache_dirty flag in flush_icache_page() and thus we enter __update_cache() that lacks support for HIGHMEM.

Signed-off-by Lars Persson <[email protected]>

---
 arch/mips/include/asm/cacheflush.h |    7 +++++--
 arch/mips/mm/cache.c               |   12 ++++++++++++
 2 files changed, 17 insertions(+), 2 deletions(-)

arch/mips/include/asm/cacheflush.h +5 -2

index 37d5cf9..723229f 100644
@@ -51,6 +51,7 @@ extern void (*flush_cache_range)(struct vm_area_struct *vma,
 	unsigned long start, unsigned long end);
 extern void (*flush_cache_page)(struct vm_area_struct *vma, unsigned long page, unsigned long pfn);
 extern void __flush_dcache_page(struct page *page);
+extern void __flush_icache_page(struct vm_area_struct *vma, struct page *page);
 
 #define ARCH_IMPLEMENTS_FLUSH_DCACHE_PAGE 1
 static inline void flush_dcache_page(struct page *page)

@@ -77,8 +78,10 @@ static inline void flush_icache_page(struct vm_area_struct *vma,
 	struct page *page)
 {
 	if (!cpu_has_ic_fills_f_dc && (vma->vm_flags & VM_EXEC) &&
-	    Page_dcache_dirty(page))
-		__flush_dcache_page(page);
+	    Page_dcache_dirty(page)) {
+		__flush_icache_page(vma, page);
+		ClearPageDcacheDirty(page);
+	}
 }
 
 extern void (*flush_icache_range)(unsigned long start, unsigned long end);

arch/mips/mm/cache.c +12 -0

index f7b91d3..77d96db 100644
@@ -119,6 +119,18 @@ void __flush_anon_page(struct page *page, unsigned long vmaddr)
 
 EXPORT_SYMBOL(__flush_anon_page);
 
+void __flush_icache_page(struct vm_area_struct *vma, struct page *page)
+{
+	unsigned long addr;
+
+	if (PageHighMem(page))
+		return;
+
+	addr = (unsigned long) page_address(page);
+	flush_data_cache_page(addr);
+}
+EXPORT_SYMBOL_GPL(__flush_icache_page);
+
 void __update_cache(struct vm_area_struct *vma, unsigned long address,
 	pte_t pte)
 {

Signature · 1 line

1.7.10.4

Ralf Baechle

2015-04-10 13:47 UTC #1 lore raw

Quoted 6 lines · Lars Persson

Commit 8b5fe5e54b47 ("MIPS: Fix race condition in lazy cache flushing.") triggered NULL pointer dereferences on systems with HIGHMEM.
The problem was caused by not clearing the PG_dcache_dirty flag in flush_icache_page() and thus we enter __update_cache() that lacks support for HIGHMEM.

Thanks, I've folded this patch into the original 8b5fe5e54b47 commit.

Ralf

Lars Persson

author

2015-04-10 14:31 UTC #2 lore raw

Ralf,

I came to think that also non-executable mappings for highmem pages could reach the flushing code in __update_cache() and trigger an OOPS.

Until the highmem patches are merged we should block highmem pages in __update_cache(). Could you add this to the patch ?

Sent from my iPhone

Quoted 14 lines

10 apr 2015 kl. 15:47 skrev Ralf Baechle <[email protected]>:
Quoted 8 lines · Lars Persson
Commit 8b5fe5e54b47 ("MIPS: Fix race condition in lazy cache flushing.") triggered NULL pointer dereferences on systems with HIGHMEM.
The problem was caused by not clearing the PG_dcache_dirty flag in flush_icache_page() and thus we enter __update_cache() that lacks support for HIGHMEM.
Thanks, I've folded this patch into the original 8b5fe5e54b47 commit.
Ralf

Steven J. Hill

2015-04-13 17:08 UTC #3 lore raw

Quoted 11 lines · Lars Persson

Ralf,
I came to think that also non-executable mappings for highmem pages could reach the flushing code in __update_cache() and trigger an OOPS.
Until the highmem patches are merged we should block highmem pages in __update_cache(). Could you add this to the patch ?
Sent from my iPhone

Did you look at the patches:

http://patchwork.linux-mips.org/patch/9284/
http://patchwork.linux-mips.org/patch/9285/

Your patch seems rather similar to what is found in these. I have been trying to get these accepted, but so far no luck.

Steve

Lars Persson

author

2015-04-14 07:30 UTC #4 lore raw

Quoted 14 lines · Steven J. Hill

Quoted 11 lines · Lars Persson
Ralf,
I came to think that also non-executable mappings for highmem pages could reach the flushing code in __update_cache() and trigger an OOPS.
Until the highmem patches are merged we should block highmem pages in __update_cache(). Could you add this to the patch ?
Sent from my iPhone
Did you look at the patches:

Quoted 7 lines

   http://patchwork.linux-mips.org/patch/9284/
   http://patchwork.linux-mips.org/patch/9285/
Your patch seems rather similar to what is found in these. I have been trying to get these accepted, but so far no luck.
Steve

Yes I would like to see these patches merged, they are the ones I referred to above as "the highmem patches". I think you solve all the highmem problems that I have observed, except for the icache coherency race condition that is the main focus of my patch.

If you rebase your patches on top of mine I would be happy to assist with test and reviewing.

- Lars