| Message ID | 57A2C8DF.7050401@linaro.org |
|---|---|
| State | New |
| Headers | show |
On 08/09/2016 01:23 PM, Catalin Marinas wrote: > On Mon, Aug 08, 2016 at 10:29:05AM -0400, David Long wrote: >> On 08/08/2016 07:13 AM, Daniel Thompson wrote: >>> On 04/08/16 05:47, David Long wrote: >>> >From b451caa1adaf1d03e08a44b5dad3fca31cebd97a Mon Sep 17 00:00:00 2001 >>>> From: "David A. Long" <dave.long@linaro.org> >>>> Date: Thu, 4 Aug 2016 00:35:33 -0400 >>>> Subject: [PATCH] arm64: Remove stack duplicating code from jprobes >>>> >>>> Because the arm64 calling standard allows stacked function arguments >>>> to be >>>> anywhere in the stack frame, do not attempt to duplicate the stack >>>> frame for >>>> jprobes handler functions. >>>> >>>> Signed-off-by: David A. Long <dave.long@linaro.org> >>>> --- >>>> Documentation/kprobes.txt | 7 +++++++ >>>> arch/arm64/include/asm/kprobes.h | 2 -- >>>> arch/arm64/kernel/probes/kprobes.c | 31 +++++-------------------------- >>>> 3 files changed, 12 insertions(+), 28 deletions(-) >>>> >>>> diff --git a/Documentation/kprobes.txt b/Documentation/kprobes.txt >>>> index 1f9b3e2..bd01839 100644 >>>> --- a/Documentation/kprobes.txt >>>> +++ b/Documentation/kprobes.txt >>>> @@ -103,6 +103,13 @@ Note that the probed function's args may be >>>> passed on the stack >>>> or in registers. The jprobe will work in either case, so long as the >>>> handler's prototype matches that of the probed function. >>>> >>>> +Note that in some architectures (e.g.: arm64) the stack copy is not >>> >>> Could sparc64 be added to this list? >>> >>> For the sparc folks who are new to the thread, we've previously >>> established that the sparc64 ABI passes large structures by >>> allocating them from the caller's stack frame and passing a pointer >>> to the stack frame (i.e. arguments may not be at top of the stack). >>> We also noticed that sparc code does not save/restore anything from >>> the stack. >> >> I was reluctant to do that in the context of late changes to v4.8 for arm64 >> but now that any changes for this are going in as a new patch it would >> indeed be useful to get involvement from sparc maintainers. > > I'm happy to take the arm64 patch for 4.8 as it's mainly a clean-up. > Whether you can mention sparc64 as well, it depends on the sparc > maintainers. You can either cc them or send the series as two patches, > one for documentation and the other for arm64. > I didn't think that was going to be possible after v4.8-rc1. I have separated the documentation and code changes. I will send out the new code-only patch (otherwise unchanged in content) momentarily. Thanks, -dl _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
diff --git a/Documentation/kprobes.txt b/Documentation/kprobes.txt index 1f9b3e2..bd01839 100644 --- a/Documentation/kprobes.txt +++ b/Documentation/kprobes.txt @@ -103,6 +103,13 @@ Note that the probed function's args may be passed on the stack or in registers. The jprobe will work in either case, so long as the handler's prototype matches that of the probed function. +Note that in some architectures (e.g.: arm64) the stack copy is not +done, as the actual location of stacked parameters may be outside of +a reasonable MAX_STACK_SIZE value and because that location cannot be +determined by the jprobes code. In this case the jprobes user must be +careful to make certain the calling signature of the function does +not cause parameters to be passed on the stack. + 1.3 Return Probes 1.3.1 How Does a Return Probe Work? diff --git a/arch/arm64/include/asm/kprobes.h b/arch/arm64/include/asm/kprobes.h index 61b4915..1737aec 100644 --- a/arch/arm64/include/asm/kprobes.h +++ b/arch/arm64/include/asm/kprobes.h @@ -22,7 +22,6 @@ #define __ARCH_WANT_KPROBES_INSN_SLOT #define MAX_INSN_SIZE 1 -#define MAX_STACK_SIZE 128 #define flush_insn_slot(p) do { } while (0) #define kretprobe_blacklist_size 0 @@ -47,7 +46,6 @@ struct kprobe_ctlblk { struct prev_kprobe prev_kprobe; struct kprobe_step_ctx ss_ctx; struct pt_regs jprobe_saved_regs; - char jprobes_stack[MAX_STACK_SIZE]; }; void arch_remove_kprobe(struct kprobe *); diff --git a/arch/arm64/kernel/probes/kprobes.c b/arch/arm64/kernel/probes/kprobes.c index bf97685..c6b0f40 100644 --- a/arch/arm64/kernel/probes/kprobes.c +++ b/arch/arm64/kernel/probes/kprobes.c @@ -41,18 +41,6 @@ DEFINE_PER_CPU(struct kprobe_ctlblk, kprobe_ctlblk); static void __kprobes post_kprobe_handler(struct kprobe_ctlblk *, struct pt_regs *); -static inline unsigned long min_stack_size(unsigned long addr) -{ - unsigned long size; - - if (on_irq_stack(addr, raw_smp_processor_id())) - size = IRQ_STACK_PTR(raw_smp_processor_id()) - addr; - else - size = (unsigned long)current_thread_info() + THREAD_START_SP - addr; - - return min(size, FIELD_SIZEOF(struct kprobe_ctlblk, jprobes_stack)); -} - static void __kprobes arch_prepare_ss_slot(struct kprobe *p) { /* prepare insn slot */ @@ -489,20 +477,15 @@ int __kprobes setjmp_pre_handler(struct kprobe *p, struct pt_regs *regs) { struct jprobe *jp = container_of(p, struct jprobe, kp); struct kprobe_ctlblk *kcb = get_kprobe_ctlblk(); - long stack_ptr = kernel_stack_pointer(regs); kcb->jprobe_saved_regs = *regs; /* - * As Linus pointed out, gcc assumes that the callee - * owns the argument space and could overwrite it, e.g. - * tailcall optimization. So, to be absolutely safe - * we also save and restore enough stack bytes to cover - * the argument area. + * Since we can't be sure where in the stack frame "stacked" + * pass-by-value arguments are stored we just don't try to + * duplicate any of the stack. Do not use jprobes on functions that + * use more than 64 bytes (after padding each to an 8 byte boundary) + * of arguments, or pass individual arguments larger than 16 bytes. */ - kasan_disable_current(); - memcpy(kcb->jprobes_stack, (void *)stack_ptr, - min_stack_size(stack_ptr)); - kasan_enable_current(); instruction_pointer_set(regs, (unsigned long) jp->entry); preempt_disable(); @@ -554,10 +537,6 @@ int __kprobes longjmp_break_handler(struct kprobe *p, struct pt_regs *regs) } unpause_graph_tracing(); *regs = kcb->jprobe_saved_regs; - kasan_disable_current(); - memcpy((void *)stack_addr, kcb->jprobes_stack, - min_stack_size(stack_addr)); - kasan_enable_current(); preempt_enable_no_resched(); return 1; }