* [RFC 0/2] Consolidate patch_instruction @ 2017-05-16 3:49 Balbir Singh 2017-05-16 3:49 ` [RFC 1/2] powerpc/lib/code-patching: Enhance code patching Balbir Singh ` (3 more replies) 0 siblings, 4 replies; 12+ messages in thread From: Balbir Singh @ 2017-05-16 3:49 UTC (permalink / raw) To: linuxppc-dev, mpe; +Cc: naveen.n.rao, ananth, Balbir Singh patch_instruction is enhanced in this RFC to support patching via a different virtual address (text_poke_area). The mapping of text_poke_area->addr is RW and not RWX. This way the mapping allows write for patching and then we tear down the mapping. The downside is that we introduce a spinlock which serializes our patching to one patch at a time. In this patchset we also consolidate instruction changes in kprobes to use patch_instruction(). Balbir Singh (2): powerpc/lib/code-patching: Enhance code patching powerpc/kprobes: Move kprobes over to patch_instruction arch/powerpc/kernel/kprobes.c | 4 +- arch/powerpc/lib/code-patching.c | 88 ++++++++++++++++++++++++++++++++++++++-- 2 files changed, 86 insertions(+), 6 deletions(-) -- 2.9.3 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [RFC 1/2] powerpc/lib/code-patching: Enhance code patching 2017-05-16 3:49 [RFC 0/2] Consolidate patch_instruction Balbir Singh @ 2017-05-16 3:49 ` Balbir Singh 2017-05-16 3:49 ` [RFC 2/2] powerpc/kprobes: Move kprobes over to patch_instruction Balbir Singh ` (2 subsequent siblings) 3 siblings, 0 replies; 12+ messages in thread From: Balbir Singh @ 2017-05-16 3:49 UTC (permalink / raw) To: linuxppc-dev, mpe; +Cc: naveen.n.rao, ananth, Balbir Singh 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. We can do per-cpu stuff as well. The downside of these patches that patch_instruction is now synchornized using a lock. 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 | 88 ++++++++++++++++++++++++++++++++++++++-- 1 file changed, 84 insertions(+), 4 deletions(-) diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c index 500b0f6..4aae016 100644 --- a/arch/powerpc/lib/code-patching.c +++ b/arch/powerpc/lib/code-patching.c @@ -16,19 +16,98 @@ #include <asm/code-patching.h> #include <linux/uaccess.h> #include <linux/kprobes.h> +#include <asm/pgtable.h> +#include <asm/tlbflush.h> +struct vm_struct *text_poke_area; +static DEFINE_RAW_SPINLOCK(text_poke_lock); -int patch_instruction(unsigned int *addr, unsigned int instr) +/* + * 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) { + text_poke_area = get_vm_area(PAGE_SIZE, VM_ALLOC); + if (!text_poke_area) { + WARN_ONCE(1, "could not create area for mapping kernel addrs" + " which allow for patching kernel code\n"); + return 0; + } + pr_info("text_poke area ready...\n"); + raw_spin_lock_init(&text_poke_lock); + 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)text_poke_area->addr, + (pfn << PAGE_SHIFT), _PAGE_KERNEL_RW | _PAGE_PRESENT); + pr_devel("Mapped addr %p with pfn %lx\n", 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); +} + +int patch_instruction(unsigned int *addr, unsigned int instr) +{ + int err; + unsigned int *dest = NULL; + unsigned long flags; + unsigned long kaddr = (unsigned long)addr; + + /* + * 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 + */ + if (!text_poke_area) { + __put_user_size(instr, addr, 4, err); + asm ("dcbst 0, %0; sync; icbi 0,%0; sync; isync" : : "r" (addr)); + return 0; + } + + raw_spin_lock_irqsave(&text_poke_lock, flags); + if (kernel_map_addr(addr)) { + err = -1; + goto out; + } + + dest = (unsigned int *)(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; sync; isync" : : "r" (dest)); + kernel_unmap_addr(text_poke_area->addr); +out: + raw_spin_unlock_irqrestore(&text_poke_lock, 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 +593,4 @@ static int __init test_code_patching(void) late_initcall(test_code_patching); #endif /* CONFIG_CODE_PATCHING_SELFTEST */ +early_initcall(setup_text_poke_area); -- 2.9.3 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [RFC 2/2] powerpc/kprobes: Move kprobes over to patch_instruction 2017-05-16 3:49 [RFC 0/2] Consolidate patch_instruction Balbir Singh 2017-05-16 3:49 ` [RFC 1/2] powerpc/lib/code-patching: Enhance code patching Balbir Singh @ 2017-05-16 3:49 ` Balbir Singh 2017-05-16 13:35 ` Naveen N. Rao 2017-05-16 5:26 ` [RFC 0/2] Consolidate patch_instruction Anshuman Khandual 2017-05-16 20:20 ` LEROY Christophe 3 siblings, 1 reply; 12+ messages in thread From: Balbir Singh @ 2017-05-16 3:49 UTC (permalink / raw) To: linuxppc-dev, mpe; +Cc: naveen.n.rao, ananth, Balbir Singh arch_arm/disarm_probe use direct assignment for copying instructions, replace them with patch_instruction Signed-off-by: Balbir Singh <bsingharora@gmail.com> --- arch/powerpc/kernel/kprobes.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/powerpc/kernel/kprobes.c b/arch/powerpc/kernel/kprobes.c index 160ae0f..5e1fa86 100644 --- a/arch/powerpc/kernel/kprobes.c +++ b/arch/powerpc/kernel/kprobes.c @@ -158,7 +158,7 @@ NOKPROBE_SYMBOL(arch_prepare_kprobe); void arch_arm_kprobe(struct kprobe *p) { - *p->addr = BREAKPOINT_INSTRUCTION; + patch_instruction(p->addr, BREAKPOINT_INSTRUCTION); flush_icache_range((unsigned long) p->addr, (unsigned long) p->addr + sizeof(kprobe_opcode_t)); } @@ -166,7 +166,7 @@ NOKPROBE_SYMBOL(arch_arm_kprobe); void arch_disarm_kprobe(struct kprobe *p) { - *p->addr = p->opcode; + patch_instruction(p->addr, BREAKPOINT_INSTRUCTION); flush_icache_range((unsigned long) p->addr, (unsigned long) p->addr + sizeof(kprobe_opcode_t)); } -- 2.9.3 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [RFC 2/2] powerpc/kprobes: Move kprobes over to patch_instruction 2017-05-16 3:49 ` [RFC 2/2] powerpc/kprobes: Move kprobes over to patch_instruction Balbir Singh @ 2017-05-16 13:35 ` Naveen N. Rao 2017-05-17 1:40 ` Balbir Singh 0 siblings, 1 reply; 12+ messages in thread From: Naveen N. Rao @ 2017-05-16 13:35 UTC (permalink / raw) To: Balbir Singh; +Cc: linuxppc-dev, mpe, ananth On 2017/05/16 01:49PM, Balbir Singh wrote: > arch_arm/disarm_probe use direct assignment for copying > instructions, replace them with patch_instruction Thanks for doing this! We will also have to convert optprobes and ftrace to use patch_instruction, but that can be done once the basic infrastructure is in. Regards, Naveen ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC 2/2] powerpc/kprobes: Move kprobes over to patch_instruction 2017-05-16 13:35 ` Naveen N. Rao @ 2017-05-17 1:40 ` Balbir Singh 2017-05-30 14:28 ` Naveen N. Rao 0 siblings, 1 reply; 12+ messages in thread From: Balbir Singh @ 2017-05-17 1:40 UTC (permalink / raw) To: Naveen N. Rao; +Cc: linuxppc-dev, mpe, ananth On Tue, 2017-05-16 at 19:05 +0530, Naveen N. Rao wrote: > On 2017/05/16 01:49PM, Balbir Singh wrote: > > arch_arm/disarm_probe use direct assignment for copying > > instructions, replace them with patch_instruction > > Thanks for doing this! > > We will also have to convert optprobes and ftrace to use > patch_instruction, but that can be done once the basic infrastructure is > in. > I think these patches can go in without even patch 1. I looked quickly at optprobes and ftrace and thought they were already using patch_instruction (ftrace_modify_code() and arch_optimize_kprobes()), are there other paths I missed? Balbir Singh ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC 2/2] powerpc/kprobes: Move kprobes over to patch_instruction 2017-05-17 1:40 ` Balbir Singh @ 2017-05-30 14:28 ` Naveen N. Rao 0 siblings, 0 replies; 12+ messages in thread From: Naveen N. Rao @ 2017-05-30 14:28 UTC (permalink / raw) To: Balbir Singh; +Cc: linuxppc-dev, mpe, ananth On 2017/05/17 11:40AM, Balbir Singh wrote: > On Tue, 2017-05-16 at 19:05 +0530, Naveen N. Rao wrote: > > On 2017/05/16 01:49PM, Balbir Singh wrote: > > > arch_arm/disarm_probe use direct assignment for copying > > > instructions, replace them with patch_instruction > > > > Thanks for doing this! > > > > We will also have to convert optprobes and ftrace to use > > patch_instruction, but that can be done once the basic infrastructure is > > in. > > > > I think these patches can go in without even patch 1. I looked quickly at > optprobes and ftrace and thought they were already using patch_instruction > (ftrace_modify_code() and arch_optimize_kprobes()), are there other paths > I missed? [Sorry for the delay...] Yes, all the patch_*_insns() functions need to be converted since the area they patch comes from .text. There is also a memcpy() where we copy the instruction template in, so perhaps a patch_instructions() helper may be useful too. - Naveen ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC 0/2] Consolidate patch_instruction 2017-05-16 3:49 [RFC 0/2] Consolidate patch_instruction Balbir Singh 2017-05-16 3:49 ` [RFC 1/2] powerpc/lib/code-patching: Enhance code patching Balbir Singh 2017-05-16 3:49 ` [RFC 2/2] powerpc/kprobes: Move kprobes over to patch_instruction Balbir Singh @ 2017-05-16 5:26 ` Anshuman Khandual 2017-05-16 13:41 ` Naveen N. Rao 2017-05-16 20:20 ` LEROY Christophe 3 siblings, 1 reply; 12+ messages in thread From: Anshuman Khandual @ 2017-05-16 5:26 UTC (permalink / raw) To: Balbir Singh, linuxppc-dev, mpe; +Cc: naveen.n.rao On 05/16/2017 09:19 AM, Balbir Singh wrote: > patch_instruction is enhanced in this RFC to support > patching via a different virtual address (text_poke_area). Why writing instruction directly into the address is not sufficient and need to go through this virtual address ? > The mapping of text_poke_area->addr is RW and not RWX. > This way the mapping allows write for patching and then we tear > down the mapping. The downside is that we introduce a spinlock > which serializes our patching to one patch at a time. So whats the benifits we get otherwise in this approach when we are adding a new lock into the equation. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC 0/2] Consolidate patch_instruction 2017-05-16 5:26 ` [RFC 0/2] Consolidate patch_instruction Anshuman Khandual @ 2017-05-16 13:41 ` Naveen N. Rao 2017-05-17 1:23 ` Balbir Singh 0 siblings, 1 reply; 12+ messages in thread From: Naveen N. Rao @ 2017-05-16 13:41 UTC (permalink / raw) To: Anshuman Khandual; +Cc: Balbir Singh, linuxppc-dev, mpe On 2017/05/16 10:56AM, Anshuman Khandual wrote: > On 05/16/2017 09:19 AM, Balbir Singh wrote: > > patch_instruction is enhanced in this RFC to support > > patching via a different virtual address (text_poke_area). > > Why writing instruction directly into the address is not > sufficient and need to go through this virtual address ? To enable KERNEL_STRICT_RWX and map all of kernel text to be read-only? > > > The mapping of text_poke_area->addr is RW and not RWX. > > This way the mapping allows write for patching and then we tear > > down the mapping. The downside is that we introduce a spinlock > > which serializes our patching to one patch at a time. > > So whats the benifits we get otherwise in this approach when > we are adding a new lock into the equation. Instruction patching isn't performance critical, so the slow down is likely not noticeable. Marking kernel text read-only helps harden the kernel by catching unintended code modifications whether through exploits or through bugs. - Naveen ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC 0/2] Consolidate patch_instruction 2017-05-16 13:41 ` Naveen N. Rao @ 2017-05-17 1:23 ` Balbir Singh 0 siblings, 0 replies; 12+ messages in thread From: Balbir Singh @ 2017-05-17 1:23 UTC (permalink / raw) To: Naveen N. Rao, Anshuman Khandual; +Cc: linuxppc-dev, mpe On Tue, 2017-05-16 at 19:11 +0530, Naveen N. Rao wrote: > On 2017/05/16 10:56AM, Anshuman Khandual wrote: > > On 05/16/2017 09:19 AM, Balbir Singh wrote: > > > patch_instruction is enhanced in this RFC to support > > > patching via a different virtual address (text_poke_area). > > > > Why writing instruction directly into the address is not > > sufficient and need to go through this virtual address ? > > To enable KERNEL_STRICT_RWX and map all of kernel text to be read-only? > Precisely, the rest of the bits are still being developed. > > > > > The mapping of text_poke_area->addr is RW and not RWX. > > > This way the mapping allows write for patching and then we tear > > > down the mapping. The downside is that we introduce a spinlock > > > which serializes our patching to one patch at a time. > > > > So whats the benifits we get otherwise in this approach when > > we are adding a new lock into the equation. > > Instruction patching isn't performance critical, so the slow down is > likely not noticeable. Marking kernel text read-only helps harden the > kernel by catching unintended code modifications whether through > exploits or through bugs. > Precisely! Balbir Singh. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC 0/2] Consolidate patch_instruction 2017-05-16 3:49 [RFC 0/2] Consolidate patch_instruction Balbir Singh ` (2 preceding siblings ...) 2017-05-16 5:26 ` [RFC 0/2] Consolidate patch_instruction Anshuman Khandual @ 2017-05-16 20:20 ` LEROY Christophe 2017-05-17 2:10 ` Balbir Singh 3 siblings, 1 reply; 12+ messages in thread From: LEROY Christophe @ 2017-05-16 20:20 UTC (permalink / raw) To: Balbir Singh; +Cc: naveen.n.rao, mpe, linuxppc-dev Balbir Singh <bsingharora@gmail.com> a =C3=A9crit=C2=A0: > patch_instruction is enhanced in this RFC to support > patching via a different virtual address (text_poke_area). > The mapping of text_poke_area->addr is RW and not RWX. > This way the mapping allows write for patching and then we tear > down the mapping. The downside is that we introduce a spinlock > which serializes our patching to one patch at a time. Very nice patch, would fit great with my patch for impmementing=20=20 CONFIG_DEBUG_RODATA=20(https://patchwork.ozlabs.org/patch/754289 ). Would avoid having to set the text area back to RW for patching Christophe > > In this patchset we also consolidate instruction changes > in kprobes to use patch_instruction(). > > Balbir Singh (2): > powerpc/lib/code-patching: Enhance code patching > powerpc/kprobes: Move kprobes over to patch_instruction > > arch/powerpc/kernel/kprobes.c | 4 +- > arch/powerpc/lib/code-patching.c | 88=20=20 >=20++++++++++++++++++++++++++++++++++++++-- > 2 files changed, 86 insertions(+), 6 deletions(-) > > -- > 2.9.3 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC 0/2] Consolidate patch_instruction 2017-05-16 20:20 ` LEROY Christophe @ 2017-05-17 2:10 ` Balbir Singh 2017-05-17 7:04 ` LEROY Christophe 0 siblings, 1 reply; 12+ messages in thread From: Balbir Singh @ 2017-05-17 2:10 UTC (permalink / raw) To: LEROY Christophe; +Cc: naveen.n.rao, mpe, linuxppc-dev On Tue, 2017-05-16 at 22:20 +0200, LEROY Christophe wrote: > Balbir Singh <bsingharora@gmail.com> a écrit : > > > patch_instruction is enhanced in this RFC to support > > patching via a different virtual address (text_poke_area). > > The mapping of text_poke_area->addr is RW and not RWX. > > This way the mapping allows write for patching and then we tear > > down the mapping. The downside is that we introduce a spinlock > > which serializes our patching to one patch at a time. > > Very nice patch, would fit great with my patch for impmementing > CONFIG_DEBUG_RODATA (https://patchwork.ozlabs.org/patch/754289 ). > Would avoid having to set the text area back to RW for patching > Awesome! It seems like you have some of the work for CONFIG_STRICT_KERNEL_RWX any reason why this is under CONFIG_DEBUG_RODATA? But I think there is reuse capability across the future patches and the current set. Cheers, Balbir Singh. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC 0/2] Consolidate patch_instruction 2017-05-17 2:10 ` Balbir Singh @ 2017-05-17 7:04 ` LEROY Christophe 0 siblings, 0 replies; 12+ messages in thread From: LEROY Christophe @ 2017-05-17 7:04 UTC (permalink / raw) To: Balbir Singh; +Cc: linuxppc-dev, mpe, naveen.n.rao Balbir Singh <bsingharora@gmail.com> a =C3=A9crit=C2=A0: > On Tue, 2017-05-16 at 22:20 +0200, LEROY Christophe wrote: >> Balbir Singh <bsingharora@gmail.com> a =C3=A9crit=C2=A0: >> >> > patch_instruction is enhanced in this RFC to support >> > patching via a different virtual address (text_poke_area). >> > The mapping of text_poke_area->addr is RW and not RWX. >> > This way the mapping allows write for patching and then we tear >> > down the mapping. The downside is that we introduce a spinlock >> > which serializes our patching to one patch at a time. >> >> Very nice patch, would fit great with my patch for impmementing >> CONFIG_DEBUG_RODATA (https://patchwork.ozlabs.org/patch/754289 ). >> Would avoid having to set the text area back to RW for patching >> > > Awesome! It seems like you have some of the work for CONFIG_STRICT_KERNEL= _RWX > any reason why this is under CONFIG_DEBUG_RODATA? But I think there is > reuse capability across the future patches and the current set. > Indeed it looks the same, see https://patchwork.kernel.org/patch/9554881 Christophe > Cheers, > Balbir Singh. ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2017-05-30 14:29 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-05-16 3:49 [RFC 0/2] Consolidate patch_instruction Balbir Singh 2017-05-16 3:49 ` [RFC 1/2] powerpc/lib/code-patching: Enhance code patching Balbir Singh 2017-05-16 3:49 ` [RFC 2/2] powerpc/kprobes: Move kprobes over to patch_instruction Balbir Singh 2017-05-16 13:35 ` Naveen N. Rao 2017-05-17 1:40 ` Balbir Singh 2017-05-30 14:28 ` Naveen N. Rao 2017-05-16 5:26 ` [RFC 0/2] Consolidate patch_instruction Anshuman Khandual 2017-05-16 13:41 ` Naveen N. Rao 2017-05-17 1:23 ` Balbir Singh 2017-05-16 20:20 ` LEROY Christophe 2017-05-17 2:10 ` Balbir Singh 2017-05-17 7:04 ` LEROY Christophe
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.