The set_memory_{ro/rw/nx/x}() functions are required for STRICT_MODULE_RWX,
and are generally useful primitives to have. This implementation is
designed to be completely generic across powerpc's many MMUs.
It's possible that this could be optimised to be faster for specific
MMUs, but the focus is on having a generic and safe implementation for
now.
This implementation does not handle cases where the caller is attempting
to change the mapping of the page it is executing from, or if another
CPU is concurrently using the page being altered. These cases likely
shouldn't happen, but a more complex implementation with MMU-specific code
could safely handle them, so that is left as a TODO for now.
Signed-off-by: Russell Currey <redacted>
Signed-off-by: Christophe Leroy <redacted>
---
v3:
- Changes 'action' from int to long to avoid build failure on PPC64 when casting to/from void*
- Move pageattr.o into obj-y in Makefile
v2:
- use integers instead of pointers for action
- drop action check, nobody should call change_memory_attr() directly.
Should it happen, the function will just do nothing.
- Renamed confusing 'pte_val' var to 'pte' as pte_val() is already a function.
Signed-off-by: Christophe Leroy <redacted>
---
arch/powerpc/Kconfig | 1 +
arch/powerpc/include/asm/set_memory.h | 32 ++++++++++++
arch/powerpc/mm/Makefile | 2 +-
arch/powerpc/mm/pageattr.c | 74 +++++++++++++++++++++++++++
4 files changed, 108 insertions(+), 1 deletion(-)
create mode 100644 arch/powerpc/include/asm/set_memory.h
create mode 100644 arch/powerpc/mm/pageattr.c
@@ -0,0 +1,74 @@+// SPDX-License-Identifier: GPL-2.0++/*+*MMU-genericset_memoryimplementationforpowerpc+*+*Copyright2019,IBMCorporation.+*/++#include<linux/mm.h>+#include<linux/set_memory.h>++#include<asm/mmu.h>+#include<asm/page.h>+#include<asm/pgtable.h>+++/*+*Updatestheattributesofapageinthreesteps:+*+*1.invalidatethepagetableentry+*2.flushtheTLB+*3.installthenewentrywiththeupdatedattributes+*+*Thisisunsafeifthecallerisattemptingtochangethemappingofthe+*pageitisexecutingfrom,orifanotherCPUisconcurrentlyusingthe+*pagebeingaltered.+*+*TODOmaketheimplementationresistanttothis.+*/+staticintchange_page_attr(pte_t*ptep,unsignedlongaddr,void*data)+{+longaction=(long)data;+pte_tpte;++spin_lock(&init_mm.page_table_lock);++/* invalidate the PTE so it's safe to modify */+pte=ptep_get_and_clear(&init_mm,addr,ptep);+flush_tlb_kernel_range(addr,addr+PAGE_SIZE);++/* modify the PTE bits as desired, then apply */+switch(action){+caseSET_MEMORY_RO:+pte=pte_wrprotect(pte);+break;+caseSET_MEMORY_RW:+pte=pte_mkwrite(pte);+break;+caseSET_MEMORY_NX:+pte=pte_exprotect(pte);+break;+caseSET_MEMORY_X:+pte=pte_mkexec(pte);+break;+default:+break;+}++set_pte_at(&init_mm,addr,ptep,pte);+spin_unlock(&init_mm.page_table_lock);++return0;+}++intchange_memory_attr(unsignedlongaddr,intnumpages,longaction)+{+unsignedlongstart=ALIGN_DOWN(addr,PAGE_SIZE);+unsignedlongsz=numpages*PAGE_SIZE;++if(!numpages)+return0;++returnapply_to_page_range(&init_mm,start,sz,change_page_attr,(void*)action);+}
With CONFIG_STRICT_KERNEL_RWX=y and CONFIG_KPROBES=y, there will be one
W+X page at boot by default. This can be tested with
CONFIG_PPC_PTDUMP=y and CONFIG_PPC_DEBUG_WX=y set, and checking the
kernel log during boot.
powerpc doesn't implement its own alloc() for kprobes like other
architectures do, but we couldn't immediately mark RO anyway since we do
a memcpy to the page we allocate later. After that, nothing should be
allowed to modify the page, and write permissions are removed well
before the kprobe is armed.
The memcpy() would fail if >1 probes were allocated, so use
patch_instruction() instead which is safe for RO.
Reviewed-by: Daniel Axtens <redacted>
Signed-off-by: Russell Currey <redacted>
Signed-off-by: Christophe Leroy <redacted>
---
v3: copied alloc_insn_page() from arm64, set_memory_ro() is now called there.
v2: removed the redundant flush
---
arch/powerpc/kernel/kprobes.c | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)
From: Russell Currey <redacted>
Very rudimentary, just
echo 1 > [debugfs]/check_wx_pages
and check the kernel log. Useful for testing strict module RWX.
Updated the Kconfig entry to reflect this.
Also fixed a typo.
Signed-off-by: Russell Currey <redacted>
---
v3: no change
v2: no change
---
arch/powerpc/Kconfig.debug | 6 ++++--
arch/powerpc/mm/ptdump/ptdump.c | 21 ++++++++++++++++++++-
2 files changed, 24 insertions(+), 3 deletions(-)
@@ -370,7 +370,7 @@ config PPC_PTDUMPIfyouareunsure,sayN.configPPC_DEBUG_WX-bool"Warn on W+X mappings at boot"+bool"Warn on W+X mappings at boot & enable manual checks at runtime"depends onPPC_PTDUMP&&STRICT_KERNEL_RWXhelpGenerateawarningifanyW+Xmappingsarefoundatboot.
From: Russell Currey <redacted>
To enable strict module RWX on powerpc, set:
CONFIG_STRICT_MODULE_RWX=y
You should also have CONFIG_STRICT_KERNEL_RWX=y set to have any real
security benefit.
ARCH_HAS_STRICT_MODULE_RWX is set to require ARCH_HAS_STRICT_KERNEL_RWX.
This is due to a quirk in arch/Kconfig and arch/powerpc/Kconfig that
makes STRICT_MODULE_RWX *on by default* in configurations where
STRICT_KERNEL_RWX is *unavailable*.
Since this doesn't make much sense, and module RWX without kernel RWX
doesn't make much sense, having the same dependencies as kernel RWX
works around this problem.
Signed-off-by: Russell Currey <redacted>
---
v3: no change
v2: no change
---
arch/powerpc/Kconfig | 1 +
1 file changed, 1 insertion(+)
From: Russell Currey <redacted>
skiroot_defconfig is the only powerpc defconfig with STRICT_KERNEL_RWX
enabled, and if you want memory protection for kernel text you'd want it
for modules too, so enable STRICT_MODULE_RWX there.
Acked-by: Joel Stanley <redacted>
Signed-off-by: Russell Currey <redacted>
---
v3: no change
v2: no change
---
arch/powerpc/configs/skiroot_defconfig | 1 +
1 file changed, 1 insertion(+)
In addition to the set_memory_xx() functions which allows to change
the memory attributes of not (yet) used memory regions, implement a
set_memory_attr() function to:
- set the final memory protection after init on currently used
kernel regions.
- enable/disable kernel memory regions in the scope of DEBUG_PAGEALLOC.
Unlike the set_memory_xx() which can act in three step as the regions
are unused, this function must modify 'on the fly' as the kernel is
executing from them. At the moment only PPC32 will use it and changing
page attributes on the fly is not an issue.
Signed-off-by: Christophe Leroy <redacted>
---
v3: no change
v2: new
---
arch/powerpc/include/asm/set_memory.h | 2 ++
arch/powerpc/mm/pageattr.c | 33 +++++++++++++++++++++++++++
2 files changed, 35 insertions(+)
@@ -29,4 +29,6 @@ static inline int set_memory_x(unsigned long addr, int numpages)returnchange_memory_attr(addr,numpages,SET_MEMORY_X);}+intset_memory_attr(unsignedlongaddr,intnumpages,pgprot_tprot);+#endif
@@ -72,3 +72,36 @@ int change_memory_attr(unsigned long addr, int numpages, long action)returnapply_to_page_range(&init_mm,start,sz,change_page_attr,(void*)action);}++/*+*Settheattributesofapage:+*+*ThisfunctionisusedbyPPC32attheendofinittosetfinalkernelmemory+*protection.Itincludeschangingthemapingofthepageitisexecutingfrom+*anddatapagesitisusing.+*/+staticintset_page_attr(pte_t*ptep,unsignedlongaddr,void*data)+{+pgprot_tprot=__pgprot((int)data);++spin_lock(&init_mm.page_table_lock);++set_pte_at(&init_mm,addr,ptep,pte_modify(*ptep,prot));+flush_tlb_kernel_range(addr,addr+PAGE_SIZE);++spin_unlock(&init_mm.page_table_lock);++return0;+}++intset_memory_attr(unsignedlongaddr,intnumpages,pgprot_tprot)+{+unsignedlongstart=ALIGN_DOWN(addr,PAGE_SIZE);+unsignedlongsz=numpages*PAGE_SIZE;++if(!numpages)+return0;++returnapply_to_page_range(&init_mm,start,sz,set_page_attr,+(void*)pgprot_val(prot));+}
Use set_memory_attr() instead of the PPC32 specific change_page_attr()
change_page_attr() was checking that the address was not mapped by
blocks and was handling highmem, but that's unneeded because the
affected pages can't be in highmem and block mapping verification
is already done by the callers.
Signed-off-by: Christophe Leroy <redacted>
---
v3: no change
v2: new
---
arch/powerpc/mm/pgtable_32.c | 95 ++++--------------------------------
1 file changed, 10 insertions(+), 85 deletions(-)
@@ -121,99 +122,20 @@ void __init mapin_ram(void)}}-/* Scan the real Linux page tables and return a PTE pointer for-*avirtualaddressinacontext.-*Returnstrue(1)ifPTEwasfound,zerootherwise.Thepointerto-*thePTEpointerisunmodifiedifPTEisnotfound.-*/-staticint-get_pteptr(structmm_struct*mm,unsignedlongaddr,pte_t**ptep,pmd_t**pmdp)-{-pgd_t*pgd;-pud_t*pud;-pmd_t*pmd;-pte_t*pte;-intretval=0;--pgd=pgd_offset(mm,addr&PAGE_MASK);-if(pgd){-pud=pud_offset(pgd,addr&PAGE_MASK);-if(pud&&pud_present(*pud)){-pmd=pmd_offset(pud,addr&PAGE_MASK);-if(pmd_present(*pmd)){-pte=pte_offset_map(pmd,addr&PAGE_MASK);-if(pte){-retval=1;-*ptep=pte;-if(pmdp)-*pmdp=pmd;-/* XXX caller needs to do pte_unmap, yuck */-}-}-}-}-return(retval);-}--staticint__change_page_attr_noflush(structpage*page,pgprot_tprot)-{-pte_t*kpte;-pmd_t*kpmd;-unsignedlongaddress;--BUG_ON(PageHighMem(page));-address=(unsignedlong)page_address(page);--if(v_block_mapped(address))-return0;-if(!get_pteptr(&init_mm,address,&kpte,&kpmd))-return-EINVAL;-__set_pte_at(&init_mm,address,kpte,mk_pte(page,prot),0);-pte_unmap(kpte);--return0;-}--/*-*Changethepageattributesofanpageinthelinearmapping.-*-*THISDOESNOTHINGWITHBATMAPPINGS,DEBUGUSEONLY-*/-staticintchange_page_attr(structpage*page,intnumpages,pgprot_tprot)-{-inti,err=0;-unsignedlongflags;-structpage*start=page;--local_irq_save(flags);-for(i=0;i<numpages;i++,page++){-err=__change_page_attr_noflush(page,prot);-if(err)-break;-}-wmb();-local_irq_restore(flags);-flush_tlb_kernel_range((unsignedlong)page_address(start),-(unsignedlong)page_address(page));-returnerr;-}-voidmark_initmem_nx(void){-structpage*page=virt_to_page(_sinittext);unsignedlongnumpages=PFN_UP((unsignedlong)_einittext)-PFN_DOWN((unsignedlong)_sinittext);if(v_block_mapped((unsignedlong)_stext+1))mmu_mark_initmem_nx();else-change_page_attr(page,numpages,PAGE_KERNEL);+set_memory_attr((unsignedlong)_sinittext,numpages,PAGE_KERNEL);}#ifdef CONFIG_STRICT_KERNEL_RWXvoidmark_rodata_ro(void){-structpage*page;unsignedlongnumpages;if(v_block_mapped((unsignedlong)_sinittext)){
@@ -222,20 +144,18 @@ void mark_rodata_ro(void)return;}-page=virt_to_page(_stext);numpages=PFN_UP((unsignedlong)_etext)-PFN_DOWN((unsignedlong)_stext);-change_page_attr(page,numpages,PAGE_KERNEL_ROX);+set_memory_attr((unsignedlong)_stext,numpages,PAGE_KERNEL_ROX);/**mark.rodataasreadonly.Use__init_beginratherthan__end_rodata*tocoverNOTESandEXCEPTION_TABLE.*/-page=virt_to_page(__start_rodata);numpages=PFN_UP((unsignedlong)__init_begin)-PFN_DOWN((unsignedlong)__start_rodata);-change_page_attr(page,numpages,PAGE_KERNEL_RO);+set_memory_attr((unsignedlong)__start_rodata,numpages,PAGE_KERNEL_RO);// mark_initmem_nx() should have already run by nowptdump_check_wx();
From: kbuild test robot <hidden> Date: 2020-02-11 07:26:32
Hi Christophe,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on powerpc/next]
[also build test ERROR on v5.6-rc1 next-20200211]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Christophe-Leroy/powerpc-mm-Implement-set_memory-routines/20200204-030618
base: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: powerpc-skiroot_defconfig (attached as .config)
compiler: powerpc64le-linux-gcc (GCC) 7.5.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.5.0 make.cross ARCH=powerpc
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <redacted>
All errors (new ones prefixed by >>):
In file included from arch/powerpc/include/asm/page.h:304:0,
from arch/powerpc/include/asm/mmu.h:132,
from arch/powerpc/include/asm/lppaca.h:47,
from arch/powerpc/include/asm/paca.h:17,
from arch/powerpc/include/asm/current.h:13,
from include/linux/thread_info.h:21,
from include/asm-generic/preempt.h:5,
from ./arch/powerpc/include/generated/asm/preempt.h:1,
from include/linux/preempt.h:78,
from include/linux/spinlock.h:51,
from include/linux/mmzone.h:8,
from include/linux/gfp.h:6,
from include/linux/mm.h:10,
from arch/powerpc/mm/pageattr.c:9:
arch/powerpc/mm/pageattr.c: In function 'set_page_attr':
quoted
arch/powerpc/mm/pageattr.c:85:27: error: cast from pointer to integer of different size [-Werror=pointer-to-int-cast]
pgprot_t prot = __pgprot((int)data);
^
arch/powerpc/include/asm/pgtable-be-types.h:69:36: note: in definition of macro '__pgprot'
#define __pgprot(x) ((pgprot_t) { (x) })
^
cc1: all warnings being treated as errors
vim +85 arch/powerpc/mm/pageattr.c
75
76 /*
77 * Set the attributes of a page:
78 *
79 * This function is used by PPC32 at the end of init to set final kernel memory
80 * protection. It includes changing the maping of the page it is executing from
81 * and data pages it is using.
82 */
83 static int set_page_attr(pte_t *ptep, unsigned long addr, void *data)
84 {
> 85 pgprot_t prot = __pgprot((int)data);
86
87 spin_lock(&init_mm.page_table_lock);
88
89 set_pte_at(&init_mm, addr, ptep, pte_modify(*ptep, prot));
90 flush_tlb_kernel_range(addr, addr + PAGE_SIZE);
91
92 spin_unlock(&init_mm.page_table_lock);
93
94 return 0;
95 }
96
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org