* [PATCH v5] ARM: uprobes need icache flush after xol write @ 2014-04-26 6:34 Victor Kamensky 2014-04-26 6:34 ` Victor Kamensky 0 siblings, 1 reply; 5+ messages in thread From: Victor Kamensky @ 2014-04-26 6:34 UTC (permalink / raw) To: linux-arm-kernel Hi, Here is proposal for ARM uprobes icache flush issue. David Long and I believe that it is the best option as short/medium term fix. Ideally it would be good to find common arch solution, but it looks it is hard goal to achieve. arch_uprobe_copy_ixol function is introduced that implements arch specific way of handling xol slot copy. In default case we have the same code as we have now for x86 and ppc. In case of ARM the xol slot flush code shares code with ARM backend of copy_to_user_page - flush_ptrace_access function. Code and new implementation of flush_uprobe_xol_access ware modified in such way that xol flush does need vma. Code was tested on Pandaboard ES with 3.15-rc2 and latest SystemTap code from git. Tested both SMP and non SMP cases. Changes since V3 [1] version (previous version): x) Propose patch as suggested solution (dropped RFC) x) Dropped "ifdef CONFIG_SMP" around preempt_enable, preempt_disable calls x) Note V4 was RFC and contained version that explored different approach. Changes since V2 [2] version: x) address Dave Long's comment about passing checkpatch x) addressed Oleg's comment and instead of arch_uprobe_flush_xol_access function use arch_uprobe_copy_ixol function that maps kernel pages, copies, and flush caches x) removed FLAG_UA_BROADCAST, during discussion on [1] it was elaborated that task executing xol single step could be migrated to another CPU, so we need to take care of remote icaches if CPU does not support remote snooping. I.e flush_uprobe_xol_access will check cache_ops_need_broadcast() and perform smp_call_function on SMP CPUs that do not support remote snooping. x) added preempt_disable/preempt_enable in arch_uprobe_copy_ixol as copy_to_user_page does. I admit that I have some guesses, but I don't completely understand why copy_to_user_page does that, so playing on safe side - added it similar to copy_to_user_page code. Thanks, Victor [1] http://lists.infradead.org/pipermail/linux-arm-kernel/2014-April/247793.html [2] http://lists.infradead.org/pipermail/linux-arm-kernel/2014-April/245743.html Victor Kamensky (1): ARM: uprobes need icache flush after xol write arch/arm/include/asm/cacheflush.h | 2 ++ arch/arm/kernel/uprobes.c | 20 ++++++++++++++++++++ arch/arm/mm/flush.c | 33 ++++++++++++++++++++++++++++----- include/linux/uprobes.h | 3 +++ kernel/events/uprobes.c | 25 +++++++++++++++++-------- 5 files changed, 70 insertions(+), 13 deletions(-) -- 1.8.1.4 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v5] ARM: uprobes need icache flush after xol write 2014-04-26 6:34 [PATCH v5] ARM: uprobes need icache flush after xol write Victor Kamensky @ 2014-04-26 6:34 ` Victor Kamensky 2014-04-27 12:21 ` Oleg Nesterov 2014-04-27 21:01 ` David Long 0 siblings, 2 replies; 5+ messages in thread From: Victor Kamensky @ 2014-04-26 6:34 UTC (permalink / raw) To: linux-arm-kernel After instruction write into xol area, on ARM V7 architecture code need to flush dcache and icache to sync them up for given set of addresses. Having just 'flush_dcache_page(page)' call is not enough - it is possible to have stale instruction sitting in icache for given xol area slot address. Introduce arch_uprobe_ixol_copy weak function that by default calls uprobes copy_to_page function and than flush_dcache_page function and on ARM define new one that handles xol slot copy in ARM specific way flush_uprobe_xol_access function shares/reuses implementation with/of flush_ptrace_access function and takes care of writing instruction to user land address space on given variety of different cache types on ARM CPUs. Because flush_uprobe_xol_access does not have vma around flush_ptrace_access was split into two parts. First that retrieves set of condition from vma and common that receives those conditions as flags. Note ARM cache flush function need kernel address through which instruction write happened, so instead of using uprobes copy_to_page function changed code to explicitly map page and do memcpy. Note arch_uprobe_copy_ixol function, in similar way as copy_to_user_page function, has preempt_disable/preempt_enable. Signed-off-by: Victor Kamensky <victor.kamensky@linaro.org> --- arch/arm/include/asm/cacheflush.h | 2 ++ arch/arm/kernel/uprobes.c | 20 ++++++++++++++++++++ arch/arm/mm/flush.c | 33 ++++++++++++++++++++++++++++----- include/linux/uprobes.h | 3 +++ kernel/events/uprobes.c | 25 +++++++++++++++++-------- 5 files changed, 70 insertions(+), 13 deletions(-) diff --git a/arch/arm/include/asm/cacheflush.h b/arch/arm/include/asm/cacheflush.h index 8b8b616..e02712a 100644 --- a/arch/arm/include/asm/cacheflush.h +++ b/arch/arm/include/asm/cacheflush.h @@ -487,4 +487,6 @@ int set_memory_rw(unsigned long addr, int numpages); int set_memory_x(unsigned long addr, int numpages); int set_memory_nx(unsigned long addr, int numpages); +void flush_uprobe_xol_access(struct page *page, unsigned long uaddr, + void *kaddr, unsigned long len); #endif diff --git a/arch/arm/kernel/uprobes.c b/arch/arm/kernel/uprobes.c index f9bacee..56adf9c 100644 --- a/arch/arm/kernel/uprobes.c +++ b/arch/arm/kernel/uprobes.c @@ -113,6 +113,26 @@ int arch_uprobe_analyze_insn(struct arch_uprobe *auprobe, struct mm_struct *mm, return 0; } +void arch_uprobe_copy_ixol(struct page *page, unsigned long vaddr, + void *src, unsigned long len) +{ + void *xol_page_kaddr = kmap_atomic(page); + void *dst = xol_page_kaddr + (vaddr & ~PAGE_MASK); + + preempt_disable(); + + /* Initialize the slot */ + memcpy(dst, src, len); + + /* flush caches (dcache/icache) */ + flush_uprobe_xol_access(page, vaddr, dst, len); + + preempt_enable(); + + kunmap_atomic(xol_page_kaddr); +} + + int arch_uprobe_pre_xol(struct arch_uprobe *auprobe, struct pt_regs *regs) { struct uprobe_task *utask = current->utask; diff --git a/arch/arm/mm/flush.c b/arch/arm/mm/flush.c index 3387e60..43d54f5 100644 --- a/arch/arm/mm/flush.c +++ b/arch/arm/mm/flush.c @@ -104,17 +104,20 @@ void flush_cache_page(struct vm_area_struct *vma, unsigned long user_addr, unsig #define flush_icache_alias(pfn,vaddr,len) do { } while (0) #endif +#define FLAG_PA_IS_EXEC 1 +#define FLAG_PA_CORE_IN_MM 2 + static void flush_ptrace_access_other(void *args) { __flush_icache_all(); } -static -void flush_ptrace_access(struct vm_area_struct *vma, struct page *page, - unsigned long uaddr, void *kaddr, unsigned long len) +static inline +void __flush_ptrace_access(struct page *page, unsigned long uaddr, void *kaddr, + unsigned long len, unsigned int flags) { if (cache_is_vivt()) { - if (cpumask_test_cpu(smp_processor_id(), mm_cpumask(vma->vm_mm))) { + if (flags & FLAG_PA_CORE_IN_MM) { unsigned long addr = (unsigned long)kaddr; __cpuc_coherent_kern_range(addr, addr + len); } @@ -128,7 +131,7 @@ void flush_ptrace_access(struct vm_area_struct *vma, struct page *page, } /* VIPT non-aliasing D-cache */ - if (vma->vm_flags & VM_EXEC) { + if (flags & FLAG_PA_IS_EXEC) { unsigned long addr = (unsigned long)kaddr; if (icache_is_vipt_aliasing()) flush_icache_alias(page_to_pfn(page), uaddr, len); @@ -140,6 +143,26 @@ void flush_ptrace_access(struct vm_area_struct *vma, struct page *page, } } +static +void flush_ptrace_access(struct vm_area_struct *vma, struct page *page, + unsigned long uaddr, void *kaddr, unsigned long len) +{ + unsigned int flags = 0; + if (cpumask_test_cpu(smp_processor_id(), mm_cpumask(vma->vm_mm))) + flags |= FLAG_PA_CORE_IN_MM; + if (vma->vm_flags & VM_EXEC) + flags |= FLAG_PA_IS_EXEC; + __flush_ptrace_access(page, uaddr, kaddr, len, flags); +} + +void flush_uprobe_xol_access(struct page *page, unsigned long uaddr, + void *kaddr, unsigned long len) +{ + unsigned int flags = FLAG_PA_CORE_IN_MM|FLAG_PA_IS_EXEC; + + __flush_ptrace_access(page, uaddr, kaddr, len, flags); +} + /* * Copy user data from/to a page which is mapped into a different * processes address space. Really, we want to allow our "user diff --git a/include/linux/uprobes.h b/include/linux/uprobes.h index edff2b9..c52f827 100644 --- a/include/linux/uprobes.h +++ b/include/linux/uprobes.h @@ -32,6 +32,7 @@ struct vm_area_struct; struct mm_struct; struct inode; struct notifier_block; +struct page; #define UPROBE_HANDLER_REMOVE 1 #define UPROBE_HANDLER_MASK 1 @@ -127,6 +128,8 @@ extern int arch_uprobe_exception_notify(struct notifier_block *self, unsigned l extern void arch_uprobe_abort_xol(struct arch_uprobe *aup, struct pt_regs *regs); extern unsigned long arch_uretprobe_hijack_return_addr(unsigned long trampoline_vaddr, struct pt_regs *regs); extern bool __weak arch_uprobe_ignore(struct arch_uprobe *aup, struct pt_regs *regs); +extern void __weak arch_uprobe_copy_ixol(struct page *page, unsigned long vaddr, + void *src, unsigned long len); #else /* !CONFIG_UPROBES */ struct uprobes_state { }; diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c index 04709b6..4968213 100644 --- a/kernel/events/uprobes.c +++ b/kernel/events/uprobes.c @@ -1296,14 +1296,8 @@ static unsigned long xol_get_insn_slot(struct uprobe *uprobe) if (unlikely(!xol_vaddr)) return 0; - /* Initialize the slot */ - copy_to_page(area->page, xol_vaddr, - &uprobe->arch.ixol, sizeof(uprobe->arch.ixol)); - /* - * We probably need flush_icache_user_range() but it needs vma. - * This should work on supported architectures too. - */ - flush_dcache_page(area->page); + arch_uprobe_copy_ixol(area->page, xol_vaddr, + &uprobe->arch.ixol, sizeof(uprobe->arch.ixol)); return xol_vaddr; } @@ -1346,6 +1340,21 @@ static void xol_free_insn_slot(struct task_struct *tsk) } } +void __weak arch_uprobe_copy_ixol(struct page *page, unsigned long vaddr, + void *src, unsigned long len) +{ + /* Initialize the slot */ + copy_to_page(page, vaddr, src, len); + + /* + * We probably need flush_icache_user_range() but it needs vma. + * This should work on most of architectures by default. If + * architecture needs to do something different it can define + * its own version of the function. + */ + flush_dcache_page(page); +} + /** * uprobe_get_swbp_addr - compute address of swbp given post-swbp regs * @regs: Reflects the saved state of the task after it has hit a breakpoint -- 1.8.1.4 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v5] ARM: uprobes need icache flush after xol write 2014-04-26 6:34 ` Victor Kamensky @ 2014-04-27 12:21 ` Oleg Nesterov 2014-04-27 21:01 ` David Long 1 sibling, 0 replies; 5+ messages in thread From: Oleg Nesterov @ 2014-04-27 12:21 UTC (permalink / raw) To: linux-arm-kernel On 04/25, Victor Kamensky wrote: > > Note ARM cache flush function need kernel address > through which instruction write happened, so instead > of using uprobes copy_to_page function changed > code to explicitly map page and do memcpy. Again, I can't review the changes in arm/, although I think there are correct. But personally I agree with this patch, we need something for v3.15, and unless Russell objects I'd vote for this change. FWIW, Acked-by: Oleg Nesterov <oleg@redhat.com> ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v5] ARM: uprobes need icache flush after xol write 2014-04-26 6:34 ` Victor Kamensky 2014-04-27 12:21 ` Oleg Nesterov @ 2014-04-27 21:01 ` David Long 2014-04-29 3:29 ` Victor Kamensky 1 sibling, 1 reply; 5+ messages in thread From: David Long @ 2014-04-27 21:01 UTC (permalink / raw) To: linux-arm-kernel On 04/26/14 02:34, Victor Kamensky wrote: > After instruction write into xol area, on ARM V7 > architecture code need to flush dcache and icache to sync > them up for given set of addresses. Having just > 'flush_dcache_page(page)' call is not enough - it is > possible to have stale instruction sitting in icache > for given xol area slot address. > > Introduce arch_uprobe_ixol_copy weak function > that by default calls uprobes copy_to_page function and > than flush_dcache_page function and on ARM define new one > that handles xol slot copy in ARM specific way > > flush_uprobe_xol_access function shares/reuses implementation > with/of flush_ptrace_access function and takes care of writing > instruction to user land address space on given variety of > different cache types on ARM CPUs. Because > flush_uprobe_xol_access does not have vma around > flush_ptrace_access was split into two parts. First that > retrieves set of condition from vma and common that receives > those conditions as flags. > > Note ARM cache flush function need kernel address > through which instruction write happened, so instead > of using uprobes copy_to_page function changed > code to explicitly map page and do memcpy. > > Note arch_uprobe_copy_ixol function, in similar way as > copy_to_user_page function, has preempt_disable/preempt_enable. > > Signed-off-by: Victor Kamensky <victor.kamensky@linaro.org> > --- > arch/arm/include/asm/cacheflush.h | 2 ++ > arch/arm/kernel/uprobes.c | 20 ++++++++++++++++++++ > arch/arm/mm/flush.c | 33 ++++++++++++++++++++++++++++----- > include/linux/uprobes.h | 3 +++ > kernel/events/uprobes.c | 25 +++++++++++++++++-------- > 5 files changed, 70 insertions(+), 13 deletions(-) > > diff --git a/arch/arm/include/asm/cacheflush.h b/arch/arm/include/asm/cacheflush.h > index 8b8b616..e02712a 100644 > --- a/arch/arm/include/asm/cacheflush.h > +++ b/arch/arm/include/asm/cacheflush.h > @@ -487,4 +487,6 @@ int set_memory_rw(unsigned long addr, int numpages); > int set_memory_x(unsigned long addr, int numpages); > int set_memory_nx(unsigned long addr, int numpages); > > +void flush_uprobe_xol_access(struct page *page, unsigned long uaddr, > + void *kaddr, unsigned long len); > #endif > diff --git a/arch/arm/kernel/uprobes.c b/arch/arm/kernel/uprobes.c > index f9bacee..56adf9c 100644 > --- a/arch/arm/kernel/uprobes.c > +++ b/arch/arm/kernel/uprobes.c > @@ -113,6 +113,26 @@ int arch_uprobe_analyze_insn(struct arch_uprobe *auprobe, struct mm_struct *mm, > return 0; > } > > +void arch_uprobe_copy_ixol(struct page *page, unsigned long vaddr, > + void *src, unsigned long len) > +{ > + void *xol_page_kaddr = kmap_atomic(page); > + void *dst = xol_page_kaddr + (vaddr & ~PAGE_MASK); > + > + preempt_disable(); > + > + /* Initialize the slot */ > + memcpy(dst, src, len); > + > + /* flush caches (dcache/icache) */ > + flush_uprobe_xol_access(page, vaddr, dst, len); > + > + preempt_enable(); > + > + kunmap_atomic(xol_page_kaddr); > +} > + > + > int arch_uprobe_pre_xol(struct arch_uprobe *auprobe, struct pt_regs *regs) > { > struct uprobe_task *utask = current->utask; > diff --git a/arch/arm/mm/flush.c b/arch/arm/mm/flush.c > index 3387e60..43d54f5 100644 > --- a/arch/arm/mm/flush.c > +++ b/arch/arm/mm/flush.c > @@ -104,17 +104,20 @@ void flush_cache_page(struct vm_area_struct *vma, unsigned long user_addr, unsig > #define flush_icache_alias(pfn,vaddr,len) do { } while (0) > #endif > > +#define FLAG_PA_IS_EXEC 1 > +#define FLAG_PA_CORE_IN_MM 2 > + > static void flush_ptrace_access_other(void *args) > { > __flush_icache_all(); > } > > -static > -void flush_ptrace_access(struct vm_area_struct *vma, struct page *page, > - unsigned long uaddr, void *kaddr, unsigned long len) > +static inline > +void __flush_ptrace_access(struct page *page, unsigned long uaddr, void *kaddr, > + unsigned long len, unsigned int flags) > { > if (cache_is_vivt()) { > - if (cpumask_test_cpu(smp_processor_id(), mm_cpumask(vma->vm_mm))) { > + if (flags & FLAG_PA_CORE_IN_MM) { > unsigned long addr = (unsigned long)kaddr; > __cpuc_coherent_kern_range(addr, addr + len); > } > @@ -128,7 +131,7 @@ void flush_ptrace_access(struct vm_area_struct *vma, struct page *page, > } > > /* VIPT non-aliasing D-cache */ > - if (vma->vm_flags & VM_EXEC) { > + if (flags & FLAG_PA_IS_EXEC) { > unsigned long addr = (unsigned long)kaddr; > if (icache_is_vipt_aliasing()) > flush_icache_alias(page_to_pfn(page), uaddr, len); > @@ -140,6 +143,26 @@ void flush_ptrace_access(struct vm_area_struct *vma, struct page *page, > } > } > > +static > +void flush_ptrace_access(struct vm_area_struct *vma, struct page *page, > + unsigned long uaddr, void *kaddr, unsigned long len) > +{ > + unsigned int flags = 0; > + if (cpumask_test_cpu(smp_processor_id(), mm_cpumask(vma->vm_mm))) > + flags |= FLAG_PA_CORE_IN_MM; > + if (vma->vm_flags & VM_EXEC) > + flags |= FLAG_PA_IS_EXEC; > + __flush_ptrace_access(page, uaddr, kaddr, len, flags); > +} > + > +void flush_uprobe_xol_access(struct page *page, unsigned long uaddr, > + void *kaddr, unsigned long len) > +{ > + unsigned int flags = FLAG_PA_CORE_IN_MM|FLAG_PA_IS_EXEC; > + > + __flush_ptrace_access(page, uaddr, kaddr, len, flags); > +} > + > /* > * Copy user data from/to a page which is mapped into a different > * processes address space. Really, we want to allow our "user > diff --git a/include/linux/uprobes.h b/include/linux/uprobes.h > index edff2b9..c52f827 100644 > --- a/include/linux/uprobes.h > +++ b/include/linux/uprobes.h > @@ -32,6 +32,7 @@ struct vm_area_struct; > struct mm_struct; > struct inode; > struct notifier_block; > +struct page; > > #define UPROBE_HANDLER_REMOVE 1 > #define UPROBE_HANDLER_MASK 1 > @@ -127,6 +128,8 @@ extern int arch_uprobe_exception_notify(struct notifier_block *self, unsigned l > extern void arch_uprobe_abort_xol(struct arch_uprobe *aup, struct pt_regs *regs); > extern unsigned long arch_uretprobe_hijack_return_addr(unsigned long trampoline_vaddr, struct pt_regs *regs); > extern bool __weak arch_uprobe_ignore(struct arch_uprobe *aup, struct pt_regs *regs); > +extern void __weak arch_uprobe_copy_ixol(struct page *page, unsigned long vaddr, > + void *src, unsigned long len); > #else /* !CONFIG_UPROBES */ > struct uprobes_state { > }; > diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c > index 04709b6..4968213 100644 > --- a/kernel/events/uprobes.c > +++ b/kernel/events/uprobes.c > @@ -1296,14 +1296,8 @@ static unsigned long xol_get_insn_slot(struct uprobe *uprobe) > if (unlikely(!xol_vaddr)) > return 0; > > - /* Initialize the slot */ > - copy_to_page(area->page, xol_vaddr, > - &uprobe->arch.ixol, sizeof(uprobe->arch.ixol)); > - /* > - * We probably need flush_icache_user_range() but it needs vma. > - * This should work on supported architectures too. > - */ > - flush_dcache_page(area->page); > + arch_uprobe_copy_ixol(area->page, xol_vaddr, > + &uprobe->arch.ixol, sizeof(uprobe->arch.ixol)); > > return xol_vaddr; > } > @@ -1346,6 +1340,21 @@ static void xol_free_insn_slot(struct task_struct *tsk) > } > } > > +void __weak arch_uprobe_copy_ixol(struct page *page, unsigned long vaddr, > + void *src, unsigned long len) > +{ > + /* Initialize the slot */ > + copy_to_page(page, vaddr, src, len); > + > + /* > + * We probably need flush_icache_user_range() but it needs vma. > + * This should work on most of architectures by default. If > + * architecture needs to do something different it can define > + * its own version of the function. > + */ > + flush_dcache_page(page); > +} > + > /** > * uprobe_get_swbp_addr - compute address of swbp given post-swbp regs > * @regs: Reflects the saved state of the task after it has hit a breakpoint > Reviewed-by: David A. Long <dave.long@linaro.org> -dl ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v5] ARM: uprobes need icache flush after xol write 2014-04-27 21:01 ` David Long @ 2014-04-29 3:29 ` Victor Kamensky 0 siblings, 0 replies; 5+ messages in thread From: Victor Kamensky @ 2014-04-29 3:29 UTC (permalink / raw) To: linux-arm-kernel Reviewed-by David L and Acked-by Oleg I've posted the patch into Russell's patch system as: http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=8043/1 Normally, I wait longer for feedback. In this case there is some sense of urgency and hope that it could make into 3.15, so I've chosen to submit it. Russell, if you don't like it at all, please let us know and/or act accordingly within your patch system. Thanks, Victor On 27 April 2014 14:01, David Long <dave.long@linaro.org> wrote: > On 04/26/14 02:34, Victor Kamensky wrote: >> >> After instruction write into xol area, on ARM V7 >> architecture code need to flush dcache and icache to sync >> them up for given set of addresses. Having just >> 'flush_dcache_page(page)' call is not enough - it is >> possible to have stale instruction sitting in icache >> for given xol area slot address. >> >> Introduce arch_uprobe_ixol_copy weak function >> that by default calls uprobes copy_to_page function and >> than flush_dcache_page function and on ARM define new one >> that handles xol slot copy in ARM specific way >> >> flush_uprobe_xol_access function shares/reuses implementation >> with/of flush_ptrace_access function and takes care of writing >> instruction to user land address space on given variety of >> different cache types on ARM CPUs. Because >> flush_uprobe_xol_access does not have vma around >> flush_ptrace_access was split into two parts. First that >> retrieves set of condition from vma and common that receives >> those conditions as flags. >> >> Note ARM cache flush function need kernel address >> through which instruction write happened, so instead >> of using uprobes copy_to_page function changed >> code to explicitly map page and do memcpy. >> >> Note arch_uprobe_copy_ixol function, in similar way as >> copy_to_user_page function, has preempt_disable/preempt_enable. >> >> Signed-off-by: Victor Kamensky <victor.kamensky@linaro.org> >> --- >> arch/arm/include/asm/cacheflush.h | 2 ++ >> arch/arm/kernel/uprobes.c | 20 ++++++++++++++++++++ >> arch/arm/mm/flush.c | 33 ++++++++++++++++++++++++++++----- >> include/linux/uprobes.h | 3 +++ >> kernel/events/uprobes.c | 25 +++++++++++++++++-------- >> 5 files changed, 70 insertions(+), 13 deletions(-) >> >> diff --git a/arch/arm/include/asm/cacheflush.h >> b/arch/arm/include/asm/cacheflush.h >> index 8b8b616..e02712a 100644 >> --- a/arch/arm/include/asm/cacheflush.h >> +++ b/arch/arm/include/asm/cacheflush.h >> @@ -487,4 +487,6 @@ int set_memory_rw(unsigned long addr, int numpages); >> int set_memory_x(unsigned long addr, int numpages); >> int set_memory_nx(unsigned long addr, int numpages); >> >> +void flush_uprobe_xol_access(struct page *page, unsigned long uaddr, >> + void *kaddr, unsigned long len); >> #endif >> diff --git a/arch/arm/kernel/uprobes.c b/arch/arm/kernel/uprobes.c >> index f9bacee..56adf9c 100644 >> --- a/arch/arm/kernel/uprobes.c >> +++ b/arch/arm/kernel/uprobes.c >> @@ -113,6 +113,26 @@ int arch_uprobe_analyze_insn(struct arch_uprobe >> *auprobe, struct mm_struct *mm, >> return 0; >> } >> >> +void arch_uprobe_copy_ixol(struct page *page, unsigned long vaddr, >> + void *src, unsigned long len) >> +{ >> + void *xol_page_kaddr = kmap_atomic(page); >> + void *dst = xol_page_kaddr + (vaddr & ~PAGE_MASK); >> + >> + preempt_disable(); >> + >> + /* Initialize the slot */ >> + memcpy(dst, src, len); >> + >> + /* flush caches (dcache/icache) */ >> + flush_uprobe_xol_access(page, vaddr, dst, len); >> + >> + preempt_enable(); >> + >> + kunmap_atomic(xol_page_kaddr); >> +} >> + >> + >> int arch_uprobe_pre_xol(struct arch_uprobe *auprobe, struct pt_regs >> *regs) >> { >> struct uprobe_task *utask = current->utask; >> diff --git a/arch/arm/mm/flush.c b/arch/arm/mm/flush.c >> index 3387e60..43d54f5 100644 >> --- a/arch/arm/mm/flush.c >> +++ b/arch/arm/mm/flush.c >> @@ -104,17 +104,20 @@ void flush_cache_page(struct vm_area_struct *vma, >> unsigned long user_addr, unsig >> #define flush_icache_alias(pfn,vaddr,len) do { } while (0) >> #endif >> >> +#define FLAG_PA_IS_EXEC 1 >> +#define FLAG_PA_CORE_IN_MM 2 >> + >> static void flush_ptrace_access_other(void *args) >> { >> __flush_icache_all(); >> } >> >> -static >> -void flush_ptrace_access(struct vm_area_struct *vma, struct page *page, >> - unsigned long uaddr, void *kaddr, unsigned long >> len) >> +static inline >> +void __flush_ptrace_access(struct page *page, unsigned long uaddr, void >> *kaddr, >> + unsigned long len, unsigned int flags) >> { >> if (cache_is_vivt()) { >> - if (cpumask_test_cpu(smp_processor_id(), >> mm_cpumask(vma->vm_mm))) { >> + if (flags & FLAG_PA_CORE_IN_MM) { >> unsigned long addr = (unsigned long)kaddr; >> __cpuc_coherent_kern_range(addr, addr + len); >> } >> @@ -128,7 +131,7 @@ void flush_ptrace_access(struct vm_area_struct *vma, >> struct page *page, >> } >> >> /* VIPT non-aliasing D-cache */ >> - if (vma->vm_flags & VM_EXEC) { >> + if (flags & FLAG_PA_IS_EXEC) { >> unsigned long addr = (unsigned long)kaddr; >> if (icache_is_vipt_aliasing()) >> flush_icache_alias(page_to_pfn(page), uaddr, len); >> @@ -140,6 +143,26 @@ void flush_ptrace_access(struct vm_area_struct *vma, >> struct page *page, >> } >> } >> >> +static >> +void flush_ptrace_access(struct vm_area_struct *vma, struct page *page, >> + unsigned long uaddr, void *kaddr, unsigned long >> len) >> +{ >> + unsigned int flags = 0; >> + if (cpumask_test_cpu(smp_processor_id(), mm_cpumask(vma->vm_mm))) >> + flags |= FLAG_PA_CORE_IN_MM; >> + if (vma->vm_flags & VM_EXEC) >> + flags |= FLAG_PA_IS_EXEC; >> + __flush_ptrace_access(page, uaddr, kaddr, len, flags); >> +} >> + >> +void flush_uprobe_xol_access(struct page *page, unsigned long uaddr, >> + void *kaddr, unsigned long len) >> +{ >> + unsigned int flags = FLAG_PA_CORE_IN_MM|FLAG_PA_IS_EXEC; >> + >> + __flush_ptrace_access(page, uaddr, kaddr, len, flags); >> +} >> + >> /* >> * Copy user data from/to a page which is mapped into a different >> * processes address space. Really, we want to allow our "user >> diff --git a/include/linux/uprobes.h b/include/linux/uprobes.h >> index edff2b9..c52f827 100644 >> --- a/include/linux/uprobes.h >> +++ b/include/linux/uprobes.h >> @@ -32,6 +32,7 @@ struct vm_area_struct; >> struct mm_struct; >> struct inode; >> struct notifier_block; >> +struct page; >> >> #define UPROBE_HANDLER_REMOVE 1 >> #define UPROBE_HANDLER_MASK 1 >> @@ -127,6 +128,8 @@ extern int arch_uprobe_exception_notify(struct >> notifier_block *self, unsigned l >> extern void arch_uprobe_abort_xol(struct arch_uprobe *aup, struct >> pt_regs *regs); >> extern unsigned long arch_uretprobe_hijack_return_addr(unsigned long >> trampoline_vaddr, struct pt_regs *regs); >> extern bool __weak arch_uprobe_ignore(struct arch_uprobe *aup, struct >> pt_regs *regs); >> +extern void __weak arch_uprobe_copy_ixol(struct page *page, unsigned long >> vaddr, >> + void *src, unsigned long len); >> #else /* !CONFIG_UPROBES */ >> struct uprobes_state { >> }; >> diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c >> index 04709b6..4968213 100644 >> --- a/kernel/events/uprobes.c >> +++ b/kernel/events/uprobes.c >> @@ -1296,14 +1296,8 @@ static unsigned long xol_get_insn_slot(struct >> uprobe *uprobe) >> if (unlikely(!xol_vaddr)) >> return 0; >> >> - /* Initialize the slot */ >> - copy_to_page(area->page, xol_vaddr, >> - &uprobe->arch.ixol, sizeof(uprobe->arch.ixol)); >> - /* >> - * We probably need flush_icache_user_range() but it needs vma. >> - * This should work on supported architectures too. >> - */ >> - flush_dcache_page(area->page); >> + arch_uprobe_copy_ixol(area->page, xol_vaddr, >> + &uprobe->arch.ixol, >> sizeof(uprobe->arch.ixol)); >> >> return xol_vaddr; >> } >> @@ -1346,6 +1340,21 @@ static void xol_free_insn_slot(struct task_struct >> *tsk) >> } >> } >> >> +void __weak arch_uprobe_copy_ixol(struct page *page, unsigned long vaddr, >> + void *src, unsigned long len) >> +{ >> + /* Initialize the slot */ >> + copy_to_page(page, vaddr, src, len); >> + >> + /* >> + * We probably need flush_icache_user_range() but it needs vma. >> + * This should work on most of architectures by default. If >> + * architecture needs to do something different it can define >> + * its own version of the function. >> + */ >> + flush_dcache_page(page); >> +} >> + >> /** >> * uprobe_get_swbp_addr - compute address of swbp given post-swbp regs >> * @regs: Reflects the saved state of the task after it has hit a >> breakpoint >> > > > Reviewed-by: David A. Long <dave.long@linaro.org> > > > -dl > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2014-04-29 3:29 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-04-26 6:34 [PATCH v5] ARM: uprobes need icache flush after xol write Victor Kamensky 2014-04-26 6:34 ` Victor Kamensky 2014-04-27 12:21 ` Oleg Nesterov 2014-04-27 21:01 ` David Long 2014-04-29 3:29 ` Victor Kamensky
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).