Message ID | 20170603071843.11966-2-bsingharora@gmail.com (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
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 to help improve the system] url: https://github.com/0day-ci/linux/commits/Balbir-Singh/Enable-STRICT_KERNEL_RWX/20170603-161759 base: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next config: powerpc-kilauea_defconfig (attached as .config) compiler: powerpc-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705 reproduce: wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree make.cross ARCH=powerpc All errors (new ones prefixed by >>): arch/powerpc/lib/code-patching.c: In function 'kernel_map_addr': >> 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 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
Hi Balbir Le 04/06/2017 à 01:45, kbuild test robot a écrit : > 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 to help improve the system] > > url: https://github.com/0day-ci/linux/commits/Balbir-Singh/Enable-STRICT_KERNEL_RWX/20170603-161759 > base: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next > config: powerpc-kilauea_defconfig (attached as .config) > compiler: powerpc-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705 > reproduce: > wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross > chmod +x ~/bin/make.cross > # save the attached .config to linux build tree > make.cross ARCH=powerpc > > All errors (new ones prefixed by >>): > > arch/powerpc/lib/code-patching.c: In function 'kernel_map_addr': >>> 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 <christophe.leroy@c-s.fr> wrote: > Hi Balbir > > Le 04/06/2017 à 01:45, kbuild test robot a écrit : >> >> 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 to >> help improve the system] >> >> url: >> https://github.com/0day-ci/linux/commits/Balbir-Singh/Enable-STRICT_KERNEL_RWX/20170603-161759 >> base: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git >> next >> config: powerpc-kilauea_defconfig (attached as .config) >> compiler: powerpc-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705 >> reproduce: >> wget >> https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O >> ~/bin/make.cross >> chmod +x ~/bin/make.cross >> # save the attached .config to linux build tree >> make.cross ARCH=powerpc >> >> All errors (new ones prefixed by >>): >> >> arch/powerpc/lib/code-patching.c: In function 'kernel_map_addr': >>>> >>>> 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 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.
Balbir Singh <bsingharora@gmail.com> writes: > On Wed, Jun 7, 2017 at 8:25 PM, Christophe LEROY > <christophe.leroy@c-s.fr> wrote: >> Le 04/06/2017 à 01:45, kbuild test robot a écrit : >>> [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 to >>> help improve the system] >>> >>> url: >>> https://github.com/0day-ci/linux/commits/Balbir-Singh/Enable-STRICT_KERNEL_RWX/20170603-161759 >>> base: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git >>> next >>> config: powerpc-kilauea_defconfig (attached as .config) >>> compiler: powerpc-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705 >>> reproduce: >>> wget >>> https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O >>> ~/bin/make.cross >>> chmod +x ~/bin/make.cross >>> # save the attached .config to linux build tree >>> make.cross ARCH=powerpc >>> >>> All errors (new ones prefixed by >>): >>> >>> arch/powerpc/lib/code-patching.c: In function 'kernel_map_addr': >>>>> >>>>> 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. > > 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
diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c index 500b0f6..eb26b16 100644 --- a/arch/powerpc/lib/code-patching.c +++ b/arch/powerpc/lib/code-patching.c @@ -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> +static DEFINE_PER_CPU(struct vm_struct *, text_poke_area); +static unsigned int text_area_patch_avail; -int patch_instruction(unsigned int *addr, unsigned int instr) +static int text_area_cpu_up(unsigned int cpu) +{ + struct vm_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); + return 0; +} + +static int text_area_cpu_down(unsigned int cpu) +{ + free_vm_area(this_cpu_read(text_poke_area)); + return 0; +} + +/* + * This is an early_initcall and early_initcalls happen at the right time + * for us, after slab is enabled and before we mark ro pages R/O. In the + * future if get_vm_area is randomized, this will be more flexible than + * fixmap + */ +static int __init setup_text_poke_area(void) { + struct vm_struct *area; + int cpu; + + 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; + /* + * The barrier here ensures the write is visible to + * patch_instruction() + */ + smp_wmb(); + pr_info("text_poke area ready...\n"); + return 0; +} + +/* + * This can be called for kernel text or a module. + */ +static int kernel_map_addr(void *addr) +{ + unsigned long pfn; int err; - __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( + (unsigned long)__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) - return err; - asm ("dcbst 0, %0; sync; icbi 0,%0; sync; isync" : : "r" (addr)); + return -1; return 0; } +static inline void kernel_unmap_addr(void *addr) +{ + pte_t *pte; + unsigned long kaddr = (unsigned long)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); +} + +int patch_instruction(unsigned int *addr, unsigned int instr) +{ + int err; + unsigned int *dest = NULL; + unsigned long flags; + unsigned long kaddr = (unsigned long)addr; + + /* + * Make sure we can see any write of text_area_patch_avail + */ + smp_rmb(); + + /* + * During early early boot patch_instruction is called + * when text_poke_area is not ready, but we still need + * to allow patching. We just do the plain old patching + * We use text_area_patch_avail, since per cpu read + * via __this_cpu_read of text_poke_area might not + * yet be available. + * TODO: Make text_area_patch_avail per cpu? + */ + if (!text_area_patch_avail) { + __put_user_size(instr, addr, 4, err); + asm ("dcbst 0, %0; sync; icbi 0,%0; sync; isync" :: "r" (addr)); + return 0; + } + + local_irq_save(flags); + if (kernel_map_addr(addr)) { + err = -1; + goto out; + } + + dest = (unsigned int *)(__this_cpu_read(text_poke_area)->addr) + + ((kaddr & ~PAGE_MASK) / sizeof(unsigned int)); + __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); + return err; +} +NOKPROBE_SYMBOL(patch_instruction); + int patch_branch(unsigned int *addr, unsigned long target, int flags) { return patch_instruction(addr, create_branch(addr, target, flags)); @@ -514,3 +642,4 @@ static int __init test_code_patching(void) late_initcall(test_code_patching); #endif /* CONFIG_CODE_PATCHING_SELFTEST */ +early_initcall(setup_text_poke_area);
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(-)