From a950549c675f2c8c504469dec7d780da8a6433dc Mon Sep 17 00:00:00 2001 From: Vineet Gupta Date: Tue, 21 May 2013 15:25:11 +0530 Subject: ARC: copy_(to|from)_user() to honor usermode-access permissions This manifested as grep failing psuedo-randomly: -------------->8--------------------- [ARCLinux]$ ip address show lo | grep inet [ARCLinux]$ ip address show lo | grep inet [ARCLinux]$ ip address show lo | grep inet [ARCLinux]$ [ARCLinux]$ ip address show lo | grep inet inet 127.0.0.1/8 scope host lo -------------->8--------------------- ARC700 MMU provides fully orthogonal permission bits per page: Ur, Uw, Ux, Kr, Kw, Kx The user mode page permission templates used to have all Kernel mode access bits enabled. This caused a tricky race condition observed with uClibc buffered file read and UNIX pipes. 1. Read access to an anon mapped page in libc .bss: write-protected zero_page mapped: TLB Entry installed with Ur + K[rwx] 2. grep calls libc:getc() -> buffered read layer calls read(2) with the internal read buffer in same .bss page. The read() call is on STDIN which has been redirected to a pipe. read(2) => sys_read() => pipe_read() => copy_to_user() 3. Since page has Kernel-write permission (despite being user-mode write-protected), copy_to_user() suceeds w/o taking a MMU TLB-Miss Exception (page-fault for ARC). core-MM is unaware that kernel erroneously wrote to the reserved read-only zero-page (BUG #1) 4. Control returns to userspace which now does a write to same .bss page Since Linux MM is not aware that page has been modified by kernel, it simply reassigns a new writable zero-init page to mapping, loosing the prior write by kernel - effectively zero'ing out the libc read buffer under the hood - hence grep doesn't see right data (BUG #2) The fix is to make all kernel-mode access permissions mirror the user-mode ones. Note that the kernel still has full access to pages, when accessed directly (w/o MMU) - this fix ensures that kernel-mode access in copy_to_from() path uses the same faulting access model as for pure user accesses to keep MM fully aware of page state. The issue is peudo-random because it only shows up if the TLB entry installed in #1 is present at the time of #3. If it is evicted out, due to TLB pressure or some-such, then copy_to_user() does take a TLB Miss Exception, with a routine write-to-anon COW processing installing a fresh page for kernel writes and also usable as it is in userspace. Further the issue was dormant for so long as it depends on where the libc internal read buffer (in .bss) is mapped at runtime. If it happens to reside in file-backed data mapping of libc (in the page-aligned slack space trailing the file backed data), loader zero padding the slack space, does the early cow page replacement, setting things up at the very beginning itself. With gcc 4.8 based builds, the libc buffer got pushed out to a real anon mapping which triggers the issue. Reported-by: Anton Kolesov Cc: # 3.9 Signed-off-by: Vineet Gupta --- arch/arc/include/asm/pgtable.h | 26 +++++++++++++++----------- arch/arc/include/asm/tlb.h | 2 +- 2 files changed, 16 insertions(+), 12 deletions(-) (limited to 'arch/arc/include') diff --git a/arch/arc/include/asm/pgtable.h b/arch/arc/include/asm/pgtable.h index 1cc4720faccb..95b1522212a7 100644 --- a/arch/arc/include/asm/pgtable.h +++ b/arch/arc/include/asm/pgtable.h @@ -57,9 +57,9 @@ #define _PAGE_ACCESSED (1<<1) /* Page is accessed (S) */ #define _PAGE_CACHEABLE (1<<2) /* Page is cached (H) */ -#define _PAGE_EXECUTE (1<<3) /* Page has user execute perm (H) */ -#define _PAGE_WRITE (1<<4) /* Page has user write perm (H) */ -#define _PAGE_READ (1<<5) /* Page has user read perm (H) */ +#define _PAGE_U_EXECUTE (1<<3) /* Page has user execute perm (H) */ +#define _PAGE_U_WRITE (1<<4) /* Page has user write perm (H) */ +#define _PAGE_U_READ (1<<5) /* Page has user read perm (H) */ #define _PAGE_K_EXECUTE (1<<6) /* Page has kernel execute perm (H) */ #define _PAGE_K_WRITE (1<<7) /* Page has kernel write perm (H) */ #define _PAGE_K_READ (1<<8) /* Page has kernel perm (H) */ @@ -72,9 +72,9 @@ /* PD1 */ #define _PAGE_CACHEABLE (1<<0) /* Page is cached (H) */ -#define _PAGE_EXECUTE (1<<1) /* Page has user execute perm (H) */ -#define _PAGE_WRITE (1<<2) /* Page has user write perm (H) */ -#define _PAGE_READ (1<<3) /* Page has user read perm (H) */ +#define _PAGE_U_EXECUTE (1<<1) /* Page has user execute perm (H) */ +#define _PAGE_U_WRITE (1<<2) /* Page has user write perm (H) */ +#define _PAGE_U_READ (1<<3) /* Page has user read perm (H) */ #define _PAGE_K_EXECUTE (1<<4) /* Page has kernel execute perm (H) */ #define _PAGE_K_WRITE (1<<5) /* Page has kernel write perm (H) */ #define _PAGE_K_READ (1<<6) /* Page has kernel perm (H) */ @@ -93,7 +93,8 @@ #endif /* Kernel allowed all permissions for all pages */ -#define _K_PAGE_PERMS (_PAGE_K_EXECUTE | _PAGE_K_WRITE | _PAGE_K_READ) +#define _K_PAGE_PERMS (_PAGE_K_EXECUTE | _PAGE_K_WRITE | _PAGE_K_READ | \ + _PAGE_GLOBAL | _PAGE_PRESENT) #ifdef CONFIG_ARC_CACHE_PAGES #define _PAGE_DEF_CACHEABLE _PAGE_CACHEABLE @@ -106,7 +107,11 @@ * -by default cached, unless config otherwise * -present in memory */ -#define ___DEF (_PAGE_PRESENT | _K_PAGE_PERMS | _PAGE_DEF_CACHEABLE) +#define ___DEF (_PAGE_PRESENT | _PAGE_DEF_CACHEABLE) + +#define _PAGE_READ (_PAGE_U_READ | _PAGE_K_READ) +#define _PAGE_WRITE (_PAGE_U_WRITE | _PAGE_K_WRITE) +#define _PAGE_EXECUTE (_PAGE_U_EXECUTE | _PAGE_K_EXECUTE) /* Set of bits not changed in pte_modify */ #define _PAGE_CHG_MASK (PAGE_MASK | _PAGE_ACCESSED | _PAGE_MODIFIED) @@ -125,11 +130,10 @@ * kernel vaddr space - visible in all addr spaces, but kernel mode only * Thus Global, all-kernel-access, no-user-access, cached */ -#define PAGE_KERNEL __pgprot(___DEF | _PAGE_GLOBAL) +#define PAGE_KERNEL __pgprot(_K_PAGE_PERMS | _PAGE_DEF_CACHEABLE) /* ioremap */ -#define PAGE_KERNEL_NO_CACHE __pgprot(_PAGE_PRESENT | _K_PAGE_PERMS | \ - _PAGE_GLOBAL) +#define PAGE_KERNEL_NO_CACHE __pgprot(_K_PAGE_PERMS) /************************************************************************** * Mapping of vm_flags (Generic VM) to PTE flags (arch specific) diff --git a/arch/arc/include/asm/tlb.h b/arch/arc/include/asm/tlb.h index 85b6df839bd7..cb0c708ca665 100644 --- a/arch/arc/include/asm/tlb.h +++ b/arch/arc/include/asm/tlb.h @@ -16,7 +16,7 @@ /* Masks for actual TLB "PD"s */ #define PTE_BITS_IN_PD0 (_PAGE_GLOBAL | _PAGE_PRESENT) #define PTE_BITS_IN_PD1 (PAGE_MASK | _PAGE_CACHEABLE | \ - _PAGE_EXECUTE | _PAGE_WRITE | _PAGE_READ | \ + _PAGE_U_EXECUTE | _PAGE_U_WRITE | _PAGE_U_READ | \ _PAGE_K_EXECUTE | _PAGE_K_WRITE | _PAGE_K_READ) #ifndef __ASSEMBLY__ -- cgit v1.2.2 From 3e87974dec5ec25a8a4852d9292db6be659164e6 Mon Sep 17 00:00:00 2001 From: Vineet Gupta Date: Wed, 22 May 2013 18:38:10 +0530 Subject: ARC: Brown paper bag bug in macro for checking cache color The VM_EXEC check in update_mmu_cache() was getting optimized away because of a stupid error in definition of macro addr_not_cache_congruent() The intention was to have the equivalent of following: if (a || (1 ? b : 0)) but we ended up with following: if (a || 1 ? b : 0) And because precedence of '||' is more that that of '?', gcc was optimizing away evaluation of Nasty Repercussions: 1. For non-aliasing configs it would mean some extraneous dcache flushes for non-code pages if U/K mappings were not congruent. 2. For aliasing config, some needed dcache flush for code pages might be missed if U/K mappings were congruent. Signed-off-by: Vineet Gupta --- arch/arc/include/asm/cacheflush.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'arch/arc/include') diff --git a/arch/arc/include/asm/cacheflush.h b/arch/arc/include/asm/cacheflush.h index 9f841af41092..7d819749478c 100644 --- a/arch/arc/include/asm/cacheflush.h +++ b/arch/arc/include/asm/cacheflush.h @@ -99,8 +99,10 @@ static inline int cache_is_vipt_aliasing(void) * checks if two addresses (after page aligning) index into same cache set */ #define addr_not_cache_congruent(addr1, addr2) \ +({ \ cache_is_vipt_aliasing() ? \ - (CACHE_COLOR(addr1) != CACHE_COLOR(addr2)) : 0 \ + (CACHE_COLOR(addr1) != CACHE_COLOR(addr2)) : 0; \ +}) #define copy_to_user_page(vma, page, vaddr, dst, src, len) \ do { \ -- cgit v1.2.2 From 006dfb3c9c44192f06093d65b3a876fa5ad1319a Mon Sep 17 00:00:00 2001 From: Vineet Gupta Date: Sun, 19 May 2013 14:06:44 +0530 Subject: ARC: Use enough bits for determining page's cache color The current code uses 2 bits for determining page's dcache color, thus sorting pages into 4 bins, whereas the aliasing dcache really has 2 bins (8k page, 64k dcache - 4 way-set-assoc). This can cause extraneous flushes - e.g. color 0 and 2. Signed-off-by: Vineet Gupta --- arch/arc/include/asm/cacheflush.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'arch/arc/include') diff --git a/arch/arc/include/asm/cacheflush.h b/arch/arc/include/asm/cacheflush.h index 7d819749478c..ef62682e8d95 100644 --- a/arch/arc/include/asm/cacheflush.h +++ b/arch/arc/include/asm/cacheflush.h @@ -93,7 +93,7 @@ static inline int cache_is_vipt_aliasing(void) #endif } -#define CACHE_COLOR(addr) (((unsigned long)(addr) >> (PAGE_SHIFT)) & 3) +#define CACHE_COLOR(addr) (((unsigned long)(addr) >> (PAGE_SHIFT)) & 1) /* * checks if two addresses (after page aligning) index into same cache set -- cgit v1.2.2 From 7bb66f6e6eecdd8e10ed3a63bd28c1e9105adc79 Mon Sep 17 00:00:00 2001 From: Vineet Gupta Date: Sat, 25 May 2013 14:04:25 +0530 Subject: ARC: lazy dcache flush broke gdb in non-aliasing configs gdbserver inserting a breakpoint ends up calling copy_user_page() for a code page. The generic version of which (non-aliasing config) didn't set the PG_arch_1 bit hence update_mmu_cache() didn't sync dcache/icache for corresponding dynamic loader code page - causing garbade to be executed. So now aliasing versions of copy_user_highpage()/clear_page() are made default. There is no significant overhead since all of special alias handling code is compiled out for non-aliasing build Signed-off-by: Vineet Gupta --- arch/arc/include/asm/page.h | 9 --------- 1 file changed, 9 deletions(-) (limited to 'arch/arc/include') diff --git a/arch/arc/include/asm/page.h b/arch/arc/include/asm/page.h index 374a35514116..ab84bf131fe1 100644 --- a/arch/arc/include/asm/page.h +++ b/arch/arc/include/asm/page.h @@ -19,13 +19,6 @@ #define clear_page(paddr) memset((paddr), 0, PAGE_SIZE) #define copy_page(to, from) memcpy((to), (from), PAGE_SIZE) -#ifndef CONFIG_ARC_CACHE_VIPT_ALIASING - -#define clear_user_page(addr, vaddr, pg) clear_page(addr) -#define copy_user_page(vto, vfrom, vaddr, pg) copy_page(vto, vfrom) - -#else /* VIPT aliasing dcache */ - struct vm_area_struct; struct page; @@ -35,8 +28,6 @@ void copy_user_highpage(struct page *to, struct page *from, unsigned long u_vaddr, struct vm_area_struct *vma); void clear_user_page(void *to, unsigned long u_vaddr, struct page *page); -#endif /* CONFIG_ARC_CACHE_VIPT_ALIASING */ - #undef STRICT_MM_TYPECHECKS #ifdef STRICT_MM_TYPECHECKS -- cgit v1.2.2