Enable STRICT_KERNEL_RWX for PPC64/BOOK3S
These patches enable RX mappings of kernel text.
rodata is mapped RX as well as a trade-off, there
are more details in the patch description
As a prerequisite for R/O text, patch_instruction
is moved over to using a separate mapping that
allows write to kernel text. xmon/ftrace/kprobes
have been moved over to work with patch_instruction
There are a few bug fixes, the updatepp and updateboltedpp
did not use flags as described in PAPR and the ptdump
utility ignored the first PFN
TODOs:
1. Radix support
2. 32 bit support
For Radix support, it should be simple, we need to revisit
the way linear mapping is done for text, avoid any 1G
maps to make sure we use 2M and then protect at that
granularity. I will send a follow-up patch to this series
adding radix support
There are patches for 32 bit support from Christophe Leroy
at http://patchwork.ozlabs.org/patch/768257/. The patches
for map_page to map_kernel_page are a pre-requisite.
Another build failure was reported, because instead
of using ARCH_HAS_SET_MEMORY as a gate for set_memory.h
inclusion, some of the infrastructure in the core kernel
uses CONFIG_STRICT_KERNEL_RWX.
Balbir Singh (9):
powerpc/lib/code-patching: Enhance code patching
powerpc/kprobes: Move kprobes over to patch_instruction
powerpc/kprobes/optprobes: Move over to patch_instruction
powerpc/xmon: Add patch_instruction supporf for xmon
powerpc/vmlinux.lds: Align __init_begin to 16M
powerpc/platform/pseries/lpar: Fix updatepp and updateboltedpp
powerpc/mm/hash: Implement mark_rodata_ro() for hash
powerpc/Kconfig: Enable STRICT_KERNEL_RWX
powerpc/mm/ptdump: Dump the first entry of the linear mapping as well
arch/powerpc/Kconfig | 1 +
arch/powerpc/include/asm/book3s/64/hash.h | 3 +
arch/powerpc/include/asm/book3s/64/radix.h | 4 +
arch/powerpc/kernel/kprobes.c | 4 +-
arch/powerpc/kernel/optprobes.c | 58 ++++++++-----
arch/powerpc/kernel/vmlinux.lds.S | 10 ++-
arch/powerpc/lib/code-patching.c | 127 ++++++++++++++++++++++++++++-
arch/powerpc/mm/dump_hashpagetable.c | 2 +-
arch/powerpc/mm/pgtable-hash64.c | 35 ++++++++
arch/powerpc/mm/pgtable-radix.c | 7 ++
arch/powerpc/mm/pgtable_64.c | 9 ++
arch/powerpc/platforms/pseries/lpar.c | 13 ++-
arch/powerpc/xmon/xmon.c | 7 +-
13 files changed, 246 insertions(+), 34 deletions(-)
--
2.9.3
Today our patching happens via direct copy and
patch_instruction. The patching code is well
contained in the sense that copying bits are limited.
While considering implementation of CONFIG_STRICT_RWX,
the first requirement is to a create another mapping
that will allow for patching. We create the window using
text_poke_area, allocated via get_vm_area(), which might
be an overkill. text_poke_area is per CPU to avoid locking
Other arches do similar things, but use fixmaps. The reason
for not using fixmaps is to make use of any randomization in
the future. The code also relies on set_pte_at and pte_clear
to do the appropriate tlb flushing.
Signed-off-by: Balbir Singh <bsingharora@gmail.com>
---
arch/powerpc/lib/code-patching.c | 137 +++++++++++++++++++++++++++++++++++++--
1 file changed, 133 insertions(+), 4 deletions(-)
@@ -12,23 +12,151 @@#include<linux/vmalloc.h>#include<linux/init.h>#include<linux/mm.h>+#include<linux/cpuhotplug.h>#include<asm/page.h>#include<asm/code-patching.h>#include<linux/uaccess.h>#include<linux/kprobes.h>+#include<asm/pgtable.h>+#include<asm/tlbflush.h>+staticDEFINE_PER_CPU(structvm_struct*,text_poke_area);+staticunsignedinttext_area_patch_avail;-intpatch_instruction(unsignedint*addr,unsignedintinstr)+staticinttext_area_cpu_up(unsignedintcpu)+{+structvm_struct*area;++area=get_vm_area(PAGE_SIZE,VM_ALLOC);+if(!area){+WARN_ONCE(1,"Failed to create text area for cpu %d\n",+cpu);+return-1;+}+this_cpu_write(text_poke_area,area);+return0;+}++staticinttext_area_cpu_down(unsignedintcpu)+{+free_vm_area(this_cpu_read(text_poke_area));+return0;+}++/*+*Thisisanearly_initcallandearly_initcallshappenattherighttime+*forus,afterslabisenabledandbeforewemarkropagesR/O.Inthe+*futureifget_vm_areaisrandomized,thiswillbemoreflexiblethan+*fixmap+*/+staticint__initsetup_text_poke_area(void){+structvm_struct*area;+intcpu;++for_each_online_cpu(cpu){+area=get_vm_area(PAGE_SIZE,VM_ALLOC);+if(!area){+WARN_ONCE(1,"Failed to create text area for cpu %d\n",+cpu);+/* Should we disable strict rwx? */+continue;+}+this_cpu_write(text_poke_area,area);+}+cpuhp_setup_state_nocalls(CPUHP_AP_ONLINE_DYN,+"powerpc/text_poke:online",text_area_cpu_up,+text_area_cpu_down);+text_area_patch_avail=1;+/*+*Thebarrierhereensuresthewriteisvisibleto+*patch_instruction()+*/+smp_wmb();+pr_info("text_poke area ready...\n");+return0;+}++/*+*Thiscanbecalledforkerneltextoramodule.+*/+staticintkernel_map_addr(void*addr)+{+unsignedlongpfn;interr;-__put_user_size(instr,addr,4,err);+if(is_vmalloc_addr(addr))+pfn=vmalloc_to_pfn(addr);+else+pfn=__pa_symbol(addr)>>PAGE_SHIFT;++err=map_kernel_page(+(unsignedlong)__this_cpu_read(text_poke_area)->addr,+(pfn<<PAGE_SHIFT),_PAGE_KERNEL_RW|_PAGE_PRESENT);+pr_devel("Mapped addr %p with pfn %lx\n",+__this_cpu_read(text_poke_area)->addr,pfn);if(err)-returnerr;-asm("dcbst 0, %0; sync; icbi 0,%0; sync; isync"::"r"(addr));+return-1;return0;}+staticinlinevoidkernel_unmap_addr(void*addr)+{+pte_t*pte;+unsignedlongkaddr=(unsignedlong)addr;++pte=pte_offset_kernel(pmd_offset(pud_offset(pgd_offset_k(kaddr),+kaddr),kaddr),kaddr);+pr_devel("clearing mm %p, pte %p, kaddr %lx\n",&init_mm,pte,kaddr);+pte_clear(&init_mm,kaddr,pte);+flush_tlb_kernel_range(kaddr,kaddr+PAGE_SIZE);+}++intpatch_instruction(unsignedint*addr,unsignedintinstr)+{+interr;+unsignedint*dest=NULL;+unsignedlongflags;+unsignedlongkaddr=(unsignedlong)addr;++/*+*Makesurewecanseeanywriteoftext_area_patch_avail+*/+smp_rmb();++/*+*Duringearlyearlybootpatch_instructioniscalled+*whentext_poke_areaisnotready,butwestillneed+*toallowpatching.Wejustdotheplainoldpatching+*Weusetext_area_patch_avail,sincepercpuread+*via__this_cpu_readoftext_poke_areamightnot+*yetbeavailable.+*TODO:Maketext_area_patch_availpercpu?+*/+if(!text_area_patch_avail){+__put_user_size(instr,addr,4,err);+asm("dcbst 0, %0; sync; icbi 0,%0; sync; isync"::"r"(addr));+return0;+}++local_irq_save(flags);+if(kernel_map_addr(addr)){+err=-1;+gotoout;+}++dest=(unsignedint*)(__this_cpu_read(text_poke_area)->addr)++((kaddr&~PAGE_MASK)/sizeof(unsignedint));+__put_user_size(instr,dest,4,err);+asm("dcbst 0, %0; sync; icbi 0,%0; icbi 0,%1; sync; isync"+::"r"(dest),"r"(addr));+kernel_unmap_addr(__this_cpu_read(text_poke_area)->addr);+out:+local_irq_restore(flags);+returnerr;+}+NOKPROBE_SYMBOL(patch_instruction);+intpatch_branch(unsignedint*addr,unsignedlongtarget,intflags){returnpatch_instruction(addr,create_branch(addr,target,flags));
With text moving to read-only migrate optprobes to using
the patch_instruction infrastructure. Without this optprobes
will fail and complain.
Signed-off-by: Balbir Singh <bsingharora@gmail.com>
---
arch/powerpc/kernel/optprobes.c | 58 ++++++++++++++++++++++++++---------------
1 file changed, 37 insertions(+), 21 deletions(-)
@@ -231,8 +237,15 @@ int arch_prepare_optimized_kprobe(struct optimized_kprobe *op, struct kprobe *p)gotoerror;/* Setup template */-memcpy(buff,optprobe_template_entry,-TMPL_END_IDX*sizeof(kprobe_opcode_t));+/* We can optimize this via patch_instruction_window later */+size=(TMPL_END_IDX*sizeof(kprobe_opcode_t))/sizeof(int);+pr_devel("Copying template to %p, size %lu\n",buff,size);+for(i=0;i<size;i++){+rc=patch_instruction((unsignedint*)buff+i,+*((unsignedint*)(optprobe_template_entry)+i));+if(rc<0)+gotoerror;+}/**Fixupthetemplatewithinstructionsto:
For CONFIG_STRICT_KERNEL_RWX align __init_begin to 16M.
We use 16M since its the larger of 2M on radix and 16M
on hash for our linear mapping. The plan is to have
.text, .rodata and everything upto __init_begin marked
as RX. Note we still have executable read only data.
We could further align read only data to another 16M
boundary, but then the linker starts using stubs and
that breaks our assembler code in head_64.S
We don't use multi PT_LOAD in PHDRS because we are
not sure if all bootloaders support them
Signed-off-by: Balbir Singh <bsingharora@gmail.com>
---
arch/powerpc/kernel/vmlinux.lds.S | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
PAPR has pp0 in bit 55, currently we assumed that bit
pp0 is bit 0 (all bits in IBM order). This patch fixes
the pp0 bits for both these routines that use H_PROTECT
Signed-off-by: Balbir Singh <bsingharora@gmail.com>
---
arch/powerpc/platforms/pseries/lpar.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
@@ -301,7 +301,7 @@ static long pSeries_lpar_hpte_updatepp(unsigned long slot,intssize,unsignedlonginv_flags){unsignedlonglpar_rc;-unsignedlongflags=(newpp&7)|H_AVPN;+unsignedlongflags;unsignedlongwant_v;want_v=hpte_encode_avpn(vpn,psize,ssize);
@@ -309,6 +309,11 @@ static long pSeries_lpar_hpte_updatepp(unsigned long slot,pr_devel(" update: avpnv=%016lx, hash=%016lx, f=%lx, psize: %d ...",want_v,slot,flags,psize);+/*+*Movepp0andsetthemask,pp0isbit55+*Weignorethekeysfornow.+*/+flags=((newpp&HPTE_R_PP0)>>55)|(newpp&7)|H_AVPN;lpar_rc=plpar_pte_protect(flags,slot,want_v);if(lpar_rc==H_NOT_FOUND){
@@ -379,7 +384,11 @@ static void pSeries_lpar_hpte_updateboltedpp(unsigned long newpp,slot=pSeries_lpar_hpte_find(vpn,psize,ssize);BUG_ON(slot==-1);-flags=newpp&7;+/*+*Movepp0andsetthemask,pp0isbit55+*Weignorethekeysfornow.+*/+flags=((newpp&HPTE_R_PP0)>>55)|(newpp&7);lpar_rc=plpar_pte_protect(flags,slot,0);BUG_ON(lpar_rc!=H_SUCCESS);
With hash we update the bolted pte to mark it read-only. We rely
on the MMU_FTR_KERNEL_RO to generate the correct permissions
for read-only text. The radix implementation just prints a warning
in this implementation
Signed-off-by: Balbir Singh <bsingharora@gmail.com>
---
arch/powerpc/include/asm/book3s/64/hash.h | 3 +++
arch/powerpc/include/asm/book3s/64/radix.h | 4 ++++
arch/powerpc/mm/pgtable-hash64.c | 35 ++++++++++++++++++++++++++++++
arch/powerpc/mm/pgtable-radix.c | 7 ++++++
arch/powerpc/mm/pgtable_64.c | 9 ++++++++
5 files changed, 58 insertions(+)
@@ -342,3 +346,34 @@ int hash__has_transparent_hugepage(void)return1;}#endif /* CONFIG_TRANSPARENT_HUGEPAGE */++#ifdef CONFIG_STRICT_KERNEL_RWX+voidhash__mark_rodata_ro(void)+{+unsignedlongstart=(unsignedlong)_stext;+unsignedlongend=(unsignedlong)__init_begin;+unsignedlongidx;+unsignedintstep,shift;+unsignedlongnewpp=PP_RXXX;++if(!mmu_has_feature(MMU_FTR_KERNEL_RO)){+pr_info("R/O rodata not supported\n");+return;+}++shift=mmu_psize_defs[mmu_linear_psize].shift;+step=1<<shift;++start=((start+step-1)>>shift)<<shift;+end=(end>>shift)<<shift;++pr_devel("marking ro start %lx, end %lx, step %x\n",+start,end,step);++for(idx=start;idx<end;idx+=step)+/* Not sure if we can do much with the return value */+mmu_hash_ops.hpte_updateboltedpp(newpp,idx,mmu_linear_psize,+mmu_kernel_ssize);++}+#endif
@@ -108,6 +108,13 @@ int radix__map_kernel_page(unsigned long ea, unsigned long pa,return0;}+#ifdef CONFIG_STRICT_KERNEL_RWX+voidradix__mark_rodata_ro(void)+{+pr_warn("Not yet implemented for radix\n");+}+#endif+staticinlinevoid__meminitprint_mapping(unsignedlongstart,unsignedlongend,unsignedlongsize)
We have the basic support in the form of patching R/O
text sections, linker scripts for extending alignment
of text data. We've also got mark_rodata_ro()
Signed-off-by: Balbir Singh <bsingharora@gmail.com>
---
arch/powerpc/Kconfig | 1 +
1 file changed, 1 insertion(+)
The check in hpte_find() should be < and not <= for PAGE_OFFSET
Signed-off-by: Balbir Singh <bsingharora@gmail.com>
---
arch/powerpc/mm/dump_hashpagetable.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
@@ -335,7 +335,7 @@ static unsigned long hpte_find(struct pg_state *st, unsigned long ea, int psize)unsignedlongrpn,lp_bits;intbase_psize=0,actual_psize=0;-if(ea<=PAGE_OFFSET)+if(ea<PAGE_OFFSET)return-1;/* Look in primary table */
From: Nicholas Piggin <npiggin@gmail.com> Date: 2017-06-04 04:22:29
On Sat, 3 Jun 2017 17:18:39 +1000
Balbir Singh [off-list ref] wrote:
For CONFIG_STRICT_KERNEL_RWX align __init_begin to 16M.
We use 16M since its the larger of 2M on radix and 16M
on hash for our linear mapping. The plan is to have
.text, .rodata and everything upto __init_begin marked
as RX. Note we still have executable read only data.
We could further align read only data to another 16M
boundary, but then the linker starts using stubs and
that breaks our assembler code in head_64.S
Is this still the case with powerpc next?
Thanks,
Nick
On Sun, Jun 4, 2017 at 2:22 PM, Nicholas Piggin [off-list ref] wrote:
On Sat, 3 Jun 2017 17:18:39 +1000
Balbir Singh [off-list ref] wrote:
quoted
For CONFIG_STRICT_KERNEL_RWX align __init_begin to 16M.
We use 16M since its the larger of 2M on radix and 16M
on hash for our linear mapping. The plan is to have
.text, .rodata and everything upto __init_begin marked
as RX. Note we still have executable read only data.
We could further align read only data to another 16M
boundary, but then the linker starts using stubs and
that breaks our assembler code in head_64.S
Is this still the case with powerpc next?
Sorry, no, I was on linux-next for testing, but I think your linker
stub patches went in very recently. I'll rebase on top and test,
but I am not sure if I want two 16M alignments and bloat the size
of vmlinux. But I'll double check what happens
Balbir Singh.
From: Nicholas Piggin <npiggin@gmail.com> Date: 2017-06-05 06:52:09
On Mon, 5 Jun 2017 08:42:40 +1000
Balbir Singh [off-list ref] wrote:
On Sun, Jun 4, 2017 at 2:22 PM, Nicholas Piggin [off-list ref] wrote:
quoted
On Sat, 3 Jun 2017 17:18:39 +1000
Balbir Singh [off-list ref] wrote:
quoted
For CONFIG_STRICT_KERNEL_RWX align __init_begin to 16M.
We use 16M since its the larger of 2M on radix and 16M
on hash for our linear mapping. The plan is to have
.text, .rodata and everything upto __init_begin marked
as RX. Note we still have executable read only data.
We could further align read only data to another 16M
boundary, but then the linker starts using stubs and
that breaks our assembler code in head_64.S
Is this still the case with powerpc next?
Sorry, no, I was on linux-next for testing, but I think your linker
stub patches went in very recently. I'll rebase on top and test,
That would be good. I'd like to make sure the linker stub workaround
does work for your case.
but I am not sure if I want two 16M alignments and bloat the size
of vmlinux. But I'll double check what happens
arch/powerpc/lib/code-patching.c:93:8: error: implicit declaration of function 'map_kernel_page' [-Werror=implicit-function-declaration]
err = map_kernel_page(
^~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
You should probably include my patch (the one renaming map_page() to
map_kernel_page() ) in as first patch in your serie in order to get a
clean serie and avoid such reports from the robot.
Christophe
vim +/map_kernel_page +93 arch/powerpc/lib/code-patching.c
87
88 if (is_vmalloc_addr(addr))
89 pfn = vmalloc_to_pfn(addr);
90 else
91 pfn = __pa_symbol(addr) >> PAGE_SHIFT;
92
> 93 err = map_kernel_page(
94 (unsigned long)__this_cpu_read(text_poke_area)->addr,
95 (pfn << PAGE_SHIFT), _PAGE_KERNEL_RW | _PAGE_PRESENT);
96 pr_devel("Mapped addr %p with pfn %lx\n",
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
On Wed, Jun 7, 2017 at 8:25 PM, Christophe LEROY
[off-list ref] wrote:
Hi Balbir
Le 04/06/2017 =C3=A0 01:45, kbuild test robot a =C3=A9crit :
quoted
Hi Balbir,
[auto build test ERROR on powerpc/next]
[also build test ERROR on v4.12-rc3 next-20170602]
[if your patch is applied to the wrong git tree, please drop us a note t=
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=3Dpowerpc
All errors (new ones prefixed by >>):
arch/powerpc/lib/code-patching.c: In function 'kernel_map_addr':
quoted
quoted
arch/powerpc/lib/code-patching.c:93:8: error: implicit declaration of
function 'map_kernel_page' [-Werror=3Dimplicit-function-declaration]
err =3D map_kernel_page(
^~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
You should probably include my patch (the one renaming map_page() to
map_kernel_page() ) in as first patch in your serie in order to get a cle=
an
serie and avoid such reports from the robot.
Christophe
Thanks for the suggestion, I did in my cover letter mention the
pre-requisite is your patch. I'll keep that in mind if there is a v4,
otherwise, we'll need to pull in your patch first and then apply this
series.
Balbir Singh.
From: Michael Ellerman <mpe@ellerman.id.au> Date: 2017-06-07 11:52:33
Balbir Singh [off-list ref] writes:
On Wed, Jun 7, 2017 at 8:25 PM, Christophe LEROY
[off-list ref] wrote:
quoted
Le 04/06/2017 =C3=A0 01:45, kbuild test robot a =C3=A9crit :
quoted
[auto build test ERROR on powerpc/next]
[also build test ERROR on v4.12-rc3 next-20170602]
[if your patch is applied to the wrong git tree, please drop us a note =
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=3Dpowerpc
All errors (new ones prefixed by >>):
arch/powerpc/lib/code-patching.c: In function 'kernel_map_addr':
quoted
quoted
arch/powerpc/lib/code-patching.c:93:8: error: implicit declaration of
function 'map_kernel_page' [-Werror=3Dimplicit-function-declaration]
err =3D map_kernel_page(
^~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
You should probably include my patch (the one renaming map_page() to
map_kernel_page() ) in as first patch in your serie in order to get a cl=
ean
quoted
serie and avoid such reports from the robot.
Thanks for the suggestion, I did in my cover letter mention the
pre-requisite is your patch.
The kbuild robot can't read cover letters :)
I'll keep that in mind if there is a v4,
otherwise, we'll need to pull in your patch first and then apply this
series.
It'll probably be in my next tomorrow anyway.
cheers