diff mbox

[v2,1/9] powerpc/lib/code-patching: Enhance code patching

Message ID 20170603071843.11966-2-bsingharora@gmail.com (mailing list archive)
State Superseded
Headers show

Commit Message

Balbir Singh June 3, 2017, 7:18 a.m. UTC
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(-)

Comments

kernel test robot June 3, 2017, 11:45 p.m. UTC | #1
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
Christophe Leroy June 7, 2017, 10:25 a.m. UTC | #2
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
Balbir Singh June 7, 2017, 11:04 a.m. UTC | #3
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.
Michael Ellerman June 7, 2017, 11:52 a.m. UTC | #4
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 mbox

Patch

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);