| Message ID | 1350242593-17761-9-git-send-email-rabin@rab.in (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sun, Oct 14, 2012 at 09:23:13PM +0200, Rabin Vincent wrote: > Add basic uprobes support for ARM. > > perf probe --exec and SystemTap's userspace probing work. The ARM > kprobes test code has also been run in a userspace harness to test the > uprobe instruction decoding. The assumption that the target code is ARM appears to be buried all over the place. Certainly this code as currently written must depend on !THUMB2_KERNEL. However, there's an underlying problem here which we'd need to solve. The kprobes code can take advantage of the fact that the kernel is all ARM or (almost) all Thumb code. So there is no support for kprobes supporting ARM and Thumb at the same time. With userspace, we don't have this luxury. With Debian armhf, Ubuntu and Linaro building Thumb-2 userspaces, it may be an increasingly common configuration independently of whether the kernel is built in Thumb-2 or not. Furthermode, there is no such thing as a pure Thumb-2 system in practice: PLTs are always ARM, for example. For uprobes to work well in userspace, both should be supported together. I only skimmed the patches fairly quickly, so apologies if you do in fact handle this -- but I don't see any evidence of it right now. Cheers ---Dave > > Caveats: > > - Thumb is not supported > - XOL abort/trap handling is not implemented > > Signed-off-by: Rabin Vincent <rabin@rab.in> > --- > arch/arm/Kconfig | 4 + > arch/arm/include/asm/ptrace.h | 6 ++ > arch/arm/include/asm/thread_info.h | 5 +- > arch/arm/include/asm/uprobes.h | 34 +++++++ > arch/arm/kernel/Makefile | 1 + > arch/arm/kernel/signal.c | 4 + > arch/arm/kernel/uprobes.c | 191 ++++++++++++++++++++++++++++++++++++ > 7 files changed, 244 insertions(+), 1 deletion(-) > create mode 100644 arch/arm/include/asm/uprobes.h > create mode 100644 arch/arm/kernel/uprobes.c > > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig > index 272c3a1..2191b61d 100644 > --- a/arch/arm/Kconfig > +++ b/arch/arm/Kconfig > @@ -168,6 +168,10 @@ config ZONE_DMA > config NEED_DMA_MAP_STATE > def_bool y > > +config ARCH_SUPPORTS_UPROBES > + depends on KPROBES > + def_bool y > + > config ARCH_HAS_DMA_SET_COHERENT_MASK > bool > > diff --git a/arch/arm/include/asm/ptrace.h b/arch/arm/include/asm/ptrace.h > index 142d6ae..297936a 100644 > --- a/arch/arm/include/asm/ptrace.h > +++ b/arch/arm/include/asm/ptrace.h > @@ -197,6 +197,12 @@ static inline long regs_return_value(struct pt_regs *regs) > > #define instruction_pointer(regs) (regs)->ARM_pc > > +static inline void instruction_pointer_set(struct pt_regs *regs, > + unsigned long val) > +{ > + instruction_pointer(regs) = val; > +} > + > #ifdef CONFIG_SMP > extern unsigned long profile_pc(struct pt_regs *regs); > #else > diff --git a/arch/arm/include/asm/thread_info.h b/arch/arm/include/asm/thread_info.h > index 8477b4c..7bedaee 100644 > --- a/arch/arm/include/asm/thread_info.h > +++ b/arch/arm/include/asm/thread_info.h > @@ -148,6 +148,7 @@ extern int vfp_restore_user_hwstate(struct user_vfp __user *, > #define TIF_SIGPENDING 0 > #define TIF_NEED_RESCHED 1 > #define TIF_NOTIFY_RESUME 2 /* callback before returning to user */ > +#define TIF_UPROBE 7 > #define TIF_SYSCALL_TRACE 8 > #define TIF_SYSCALL_AUDIT 9 > #define TIF_SYSCALL_TRACEPOINT 10 > @@ -160,6 +161,7 @@ extern int vfp_restore_user_hwstate(struct user_vfp __user *, > #define _TIF_SIGPENDING (1 << TIF_SIGPENDING) > #define _TIF_NEED_RESCHED (1 << TIF_NEED_RESCHED) > #define _TIF_NOTIFY_RESUME (1 << TIF_NOTIFY_RESUME) > +#define _TIF_UPROBE (1 << TIF_UPROBE) > #define _TIF_SYSCALL_TRACE (1 << TIF_SYSCALL_TRACE) > #define _TIF_SYSCALL_AUDIT (1 << TIF_SYSCALL_AUDIT) > #define _TIF_SYSCALL_TRACEPOINT (1 << TIF_SYSCALL_TRACEPOINT) > @@ -172,7 +174,8 @@ extern int vfp_restore_user_hwstate(struct user_vfp __user *, > /* > * Change these and you break ASM code in entry-common.S > */ > -#define _TIF_WORK_MASK (_TIF_NEED_RESCHED | _TIF_SIGPENDING | _TIF_NOTIFY_RESUME) > +#define _TIF_WORK_MASK (_TIF_NEED_RESCHED | _TIF_SIGPENDING | \ > + _TIF_NOTIFY_RESUME | _TIF_UPROBE) > > #endif /* __KERNEL__ */ > #endif /* __ASM_ARM_THREAD_INFO_H */ > diff --git a/arch/arm/include/asm/uprobes.h b/arch/arm/include/asm/uprobes.h > new file mode 100644 > index 0000000..fa4b81e > --- /dev/null > +++ b/arch/arm/include/asm/uprobes.h > @@ -0,0 +1,34 @@ > +#ifndef _ASM_UPROBES_H > +#define _ASM_UPROBES_H > + > +#include <asm/probes.h> > + > +typedef u32 uprobe_opcode_t; > + > +#define MAX_UINSN_BYTES 4 > +#define UPROBE_XOL_SLOT_BYTES 64 > + > +#define UPROBE_SWBP_INSN 0x07f001f9 > +#define UPROBE_SS_INSN 0x07f001fa > +#define UPROBE_SWBP_INSN_SIZE 4 > + > +struct arch_uprobe_task { > + u32 backup; > +}; > + > +struct arch_uprobe { > + u8 insn[MAX_UINSN_BYTES]; > + uprobe_opcode_t modinsn; > + uprobe_opcode_t bpinsn; > + bool simulate; > + u32 pcreg; > + void (*prehandler)(struct arch_uprobe *auprobe, > + struct arch_uprobe_task *autask, > + struct pt_regs *regs); > + void (*posthandler)(struct arch_uprobe *auprobe, > + struct arch_uprobe_task *autask, > + struct pt_regs *regs); > + struct arch_specific_insn asi; > +}; > + > +#endif > diff --git a/arch/arm/kernel/Makefile b/arch/arm/kernel/Makefile > index 5bbec7b..a39f634 100644 > --- a/arch/arm/kernel/Makefile > +++ b/arch/arm/kernel/Makefile > @@ -40,6 +40,7 @@ obj-$(CONFIG_DYNAMIC_FTRACE) += ftrace.o insn.o > obj-$(CONFIG_FUNCTION_GRAPH_TRACER) += ftrace.o insn.o > obj-$(CONFIG_JUMP_LABEL) += jump_label.o insn.o patch.o > obj-$(CONFIG_KEXEC) += machine_kexec.o relocate_kernel.o > +obj-$(CONFIG_UPROBES) += uprobes.o uprobes-arm.o kprobes-arm.o > obj-$(CONFIG_KPROBES) += kprobes.o kprobes-common.o patch.o > ifdef CONFIG_THUMB2_KERNEL > obj-$(CONFIG_KPROBES) += kprobes-thumb.o > diff --git a/arch/arm/kernel/signal.c b/arch/arm/kernel/signal.c > index 56f72d2..3b1e88e 100644 > --- a/arch/arm/kernel/signal.c > +++ b/arch/arm/kernel/signal.c > @@ -12,6 +12,7 @@ > #include <linux/personality.h> > #include <linux/uaccess.h> > #include <linux/tracehook.h> > +#include <linux/uprobes.h> > > #include <asm/elf.h> > #include <asm/cacheflush.h> > @@ -655,6 +656,9 @@ do_work_pending(struct pt_regs *regs, unsigned int thread_flags, int syscall) > return restart; > } > syscall = 0; > + } else if (thread_flags & _TIF_UPROBE) { > + clear_thread_flag(TIF_UPROBE); > + uprobe_notify_resume(regs); > } else { > clear_thread_flag(TIF_NOTIFY_RESUME); > tracehook_notify_resume(regs); > diff --git a/arch/arm/kernel/uprobes.c b/arch/arm/kernel/uprobes.c > new file mode 100644 > index 0000000..f25a4af > --- /dev/null > +++ b/arch/arm/kernel/uprobes.c > @@ -0,0 +1,191 @@ > +/* > + * Copyright (C) 2012 Rabin Vincent <rabin@rab.in> > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + */ > + > +#include <linux/kernel.h> > +#include <linux/sched.h> > +#include <linux/uprobes.h> > +#include <linux/notifier.h> > +#include <linux/kprobes.h> > + > +#include <asm/opcodes.h> > +#include <asm/traps.h> > + > +#include "kprobes.h" > + > +bool is_swbp_insn(uprobe_opcode_t *insn) > +{ > + return (__mem_to_opcode_arm(*insn) & 0x0fffffff) == UPROBE_SWBP_INSN; > +} > + > +bool arch_uprobe_ignore(struct arch_uprobe *auprobe, struct pt_regs *regs) > +{ > + if (!auprobe->asi.insn_check_cc(regs->ARM_cpsr)) { > + regs->ARM_pc += 4; > + return true; > + } > + > + return false; > +} > + > +bool arch_uprobe_skip_sstep(struct arch_uprobe *auprobe, struct pt_regs *regs) > +{ > + struct kprobe kp; > + > + if (!auprobe->simulate) > + return false; > + > + kp.addr = (void *) regs->ARM_pc; > + kp.opcode = __mem_to_opcode_arm(*(unsigned int *) auprobe->insn); > + kp.ainsn.insn_handler = auprobe->asi.insn_handler; > + > + auprobe->asi.insn_singlestep(&kp, regs); > + > + return true; > +} > + > +int arch_uprobe_analyze_insn(struct arch_uprobe *auprobe, struct mm_struct *mm, > + unsigned long addr) > +{ > + unsigned int insn; > + unsigned int bpinsn; > + enum kprobe_insn ret; > + > + /* Thumb not yet support */ > + if (addr & 0x3) > + return -EINVAL; > + > + insn = __mem_to_opcode_arm(*(unsigned int *)auprobe->insn); > + auprobe->modinsn = insn; > + > + ret = arm_kprobe_decode_insn(insn, &auprobe->asi, true); > + switch (ret) { > + case INSN_REJECTED: > + return -EINVAL; > + > + case INSN_GOOD_NO_SLOT: > + auprobe->simulate = true; > + break; > + > + case INSN_GOOD: > + default: > + break; > + } > + > + bpinsn = UPROBE_SWBP_INSN; > + if (insn >= 0xe0000000) > + bpinsn |= 0xe0000000; /* Unconditional instruction */ > + else > + bpinsn |= insn & 0xf0000000; /* Copy condition from insn */ > + > + auprobe->bpinsn = bpinsn; > + > + return 0; > +} > + > +void arch_uprobe_write_opcode(struct arch_uprobe *auprobe, void *vaddr, > + uprobe_opcode_t opcode) > +{ > + unsigned long *addr = vaddr; > + > + if (opcode == UPROBE_SWBP_INSN) > + opcode = __opcode_to_mem_arm(auprobe->bpinsn); > + > + *addr = opcode; > +} > + > +void arch_uprobe_xol_copy(struct arch_uprobe *auprobe, void *vaddr) > +{ > + unsigned long *addr = vaddr; > + > + addr[0] = __opcode_to_mem_arm(auprobe->modinsn); > + addr[1] = __opcode_to_mem_arm(0xe0000000 | UPROBE_SS_INSN); > +} > + > +int arch_uprobe_pre_xol(struct arch_uprobe *auprobe, struct pt_regs *regs) > +{ > + struct uprobe_task *utask = current->utask; > + > + if (auprobe->prehandler) > + auprobe->prehandler(auprobe, &utask->autask, regs); > + > + regs->ARM_pc = utask->xol_vaddr; > + > + return 0; > +} > + > +int arch_uprobe_post_xol(struct arch_uprobe *auprobe, struct pt_regs *regs) > +{ > + struct uprobe_task *utask = current->utask; > + > + regs->ARM_pc = utask->vaddr + 4; > + > + if (auprobe->posthandler) > + auprobe->posthandler(auprobe, &utask->autask, regs); > + > + return 0; > +} > + > +bool arch_uprobe_xol_was_trapped(struct task_struct *t) > +{ > + /* TODO: implement */ > + return false; > +} > + > +void arch_uprobe_abort_xol(struct arch_uprobe *auprobe, struct pt_regs *regs) > +{ > + /* TODO: implement */ > +} > + > +int arch_uprobe_exception_notify(struct notifier_block *self, > + unsigned long val, void *data) > +{ > + return NOTIFY_DONE; > +} > + > +static int uprobe_trap_handler(struct pt_regs *regs, unsigned int instr) > +{ > + unsigned long flags; > + > + local_irq_save(flags); > + if ((instr & 0x0fffffff) == UPROBE_SWBP_INSN) > + uprobe_pre_sstep_notifier(regs); > + else > + uprobe_post_sstep_notifier(regs); > + local_irq_restore(flags); > + > + return 0; > +} > + > +unsigned long uprobe_get_swbp_addr(struct pt_regs *regs) > +{ > + return instruction_pointer(regs); > +} > + > +static struct undef_hook uprobes_arm_break_hook = { > + .instr_mask = 0x0fffffff, > + .instr_val = UPROBE_SWBP_INSN, > + .cpsr_mask = MODE_MASK, > + .cpsr_val = USR_MODE, > + .fn = uprobe_trap_handler, > +}; > + > +static struct undef_hook uprobes_arm_ss_hook = { > + .instr_mask = 0x0fffffff, > + .instr_val = UPROBE_SS_INSN, > + .cpsr_mask = MODE_MASK, > + .cpsr_val = USR_MODE, > + .fn = uprobe_trap_handler, > +}; > + > +int arch_uprobes_init(void) > +{ > + register_undef_hook(&uprobes_arm_break_hook); > + register_undef_hook(&uprobes_arm_ss_hook); > + > + return 0; > +} > -- > 1.7.9.5 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
2012/10/15 Dave Martin <dave.martin@linaro.org>: > On Sun, Oct 14, 2012 at 09:23:13PM +0200, Rabin Vincent wrote: >> Add basic uprobes support for ARM. >> >> perf probe --exec and SystemTap's userspace probing work. The ARM >> kprobes test code has also been run in a userspace harness to test the >> uprobe instruction decoding. > > The assumption that the target code is ARM appears to be buried all over > the place. Right, as stated: >> Caveats: >> - Thumb is not supported > Certainly this code as currently written must depend on !THUMB2_KERNEL. Why? It currently works for ARM userspace even if the kernel is Thumb-2. > However, there's an underlying problem here which we'd need to solve. > The kprobes code can take advantage of the fact that the kernel is all > ARM or (almost) all Thumb code. So there is no support for kprobes > supporting ARM and Thumb at the same time. > > With userspace, we don't have this luxury. With Debian armhf, Ubuntu > and Linaro building Thumb-2 userspaces, it may be an increasingly common > configuration independently of whether the kernel is built in Thumb-2 > or not. > > Furthermode, there is no such thing as a pure Thumb-2 system in practice: > PLTs are always ARM, for example. For uprobes to work well in userspace, > both should be supported together. Right. I don't think it's difficult to support both of them together, my thought was just to have userspace tell us if they want to probe a Thumb instruction via the usual 0th-bit set convention, and then take it from there. I didn't do the Thumb handling currently because I didn't really look into modifying the kprobes Thumb instruction decoding and the IT handling for uprobes.
On Sun, Oct 14, 2012 at 09:23:13PM +0200, Rabin Vincent wrote: > Add basic uprobes support for ARM. > > perf probe --exec and SystemTap's userspace probing work. The ARM > kprobes test code has also been run in a userspace harness to test the > uprobe instruction decoding. > > Caveats: > > - Thumb is not supported > - XOL abort/trap handling is not implemented [...] > diff --git a/arch/arm/kernel/uprobes.c b/arch/arm/kernel/uprobes.c > new file mode 100644 > index 0000000..f25a4af > --- /dev/null > +++ b/arch/arm/kernel/uprobes.c [...] > +bool is_swbp_insn(uprobe_opcode_t *insn) > +{ > + return (__mem_to_opcode_arm(*insn) & 0x0fffffff) == UPROBE_SWBP_INSN; You should take care not to match any instruction whose top bits are 0xF0000000. That could be some completely different instruction. [...] > +static int uprobe_trap_handler(struct pt_regs *regs, unsigned int instr) > +{ > + unsigned long flags; > + > + local_irq_save(flags); > + if ((instr & 0x0fffffff) == UPROBE_SWBP_INSN) Is the check unnecessary here? I think the same comparison will happen as a result of evaluating the associated undef_hook. However, as above you must still check for and reject cases where (instr & 0xF0000000) == 0xF0000000. [...] > +static struct undef_hook uprobes_arm_break_hook = { > + .instr_mask = 0x0fffffff, > + .instr_val = UPROBE_SWBP_INSN, > + .cpsr_mask = MODE_MASK, > + .cpsr_val = USR_MODE, > + .fn = uprobe_trap_handler, > +}; > + > +static struct undef_hook uprobes_arm_ss_hook = { > + .instr_mask = 0x0fffffff, > + .instr_val = UPROBE_SS_INSN, > + .cpsr_mask = MODE_MASK, > + .cpsr_val = USR_MODE, > + .fn = uprobe_trap_handler, > +};
On Mon, Oct 15, 2012 at 01:44:55PM +0200, Rabin Vincent wrote: > 2012/10/15 Dave Martin <dave.martin@linaro.org>: > > On Sun, Oct 14, 2012 at 09:23:13PM +0200, Rabin Vincent wrote: > >> Add basic uprobes support for ARM. > >> > >> perf probe --exec and SystemTap's userspace probing work. The ARM > >> kprobes test code has also been run in a userspace harness to test the > >> uprobe instruction decoding. > > > > The assumption that the target code is ARM appears to be buried all over > > the place. > > Right, as stated: > > >> Caveats: > >> - Thumb is not supported > > > Certainly this code as currently written must depend on !THUMB2_KERNEL. > > Why? It currently works for ARM userspace even if the kernel is > Thumb-2. My bad, I misread what was happening in the Makefile changes. My concern is about whether we can build the ARM and Thumb-2 kprobes code into the same kernel. If so, no problem, but I believe this is not a tested configuration for kprobes itself. If you've not already done so, it should be possible to test this by adding CONFIG_THUMB2_KERNEL=y to your config, providing your hardware is Thumb-2 capable. > > However, there's an underlying problem here which we'd need to solve. > > The kprobes code can take advantage of the fact that the kernel is all > > ARM or (almost) all Thumb code. So there is no support for kprobes > > supporting ARM and Thumb at the same time. > > > > With userspace, we don't have this luxury. With Debian armhf, Ubuntu > > and Linaro building Thumb-2 userspaces, it may be an increasingly common > > configuration independently of whether the kernel is built in Thumb-2 > > or not. > > > > Furthermode, there is no such thing as a pure Thumb-2 system in practice: > > PLTs are always ARM, for example. For uprobes to work well in userspace, > > both should be supported together. > > Right. I don't think it's difficult to support both of them together, > my thought was just to have userspace tell us if they want to probe a > Thumb instruction via the usual 0th-bit set convention, and then take it > from there. I didn't do the Thumb handling currently because I didn't > really look into modifying the kprobes Thumb instruction decoding and > the IT handling for uprobes. Having looked at the code a little more closely, it looks more closely aligned with this goal than I thought at first. I agree with your suggestion to require the caller to specify explicitly whether they want to insert an ARM or Thumb probe (indeed, I think there's no other way to do it, since guessing the target instruction set can't be done reliably). The code for setting a probe can then switch trivially on that low bit. Additional undef_hooks would be needed for the Thumb case, but this looks relatively straightforward, as you say. General question which I'm not sure I understand yet: is is possible to combine uprobes/kprobes decode more completely? It's not obvious to me whether the uprobes-specific decoding only relates to features which architecturally execute differently in user mode versus privileged mode. Some explanation somewhere could be helpful. Cheers ---Dave
On Mon, 2012-10-15 at 18:44 +0100, Dave Martin wrote: > On Mon, Oct 15, 2012 at 01:44:55PM +0200, Rabin Vincent wrote: > > 2012/10/15 Dave Martin <dave.martin@linaro.org>: > > > On Sun, Oct 14, 2012 at 09:23:13PM +0200, Rabin Vincent wrote: > > >> Add basic uprobes support for ARM. > > >> > > >> perf probe --exec and SystemTap's userspace probing work. The ARM > > >> kprobes test code has also been run in a userspace harness to test the > > >> uprobe instruction decoding. > > > > > > The assumption that the target code is ARM appears to be buried all over > > > the place. > > > > Right, as stated: > > > > >> Caveats: > > >> - Thumb is not supported > > > > > Certainly this code as currently written must depend on !THUMB2_KERNEL. > > > > Why? It currently works for ARM userspace even if the kernel is > > Thumb-2. > > My bad, I misread what was happening in the Makefile changes. > > My concern is about whether we can build the ARM and Thumb-2 kprobes > code into the same kernel. If so, no problem, but I believe this is > not a tested configuration for kprobes itself. When reworking kprobes I originally started by having ARM instruction support in Thumb kernels (with all test cases working) and, if I remember correct, this got dropped because we had difficulty in coming up with a robust way of specifying whether a probe pointer was in Thumb code or not. (Different methods of getting pointers to Thumb code didn't always set bit 0 so we 'solved' this by ignoring bit 0 and assuming all code in Thumb kernels was Thumb.) <snip> > General question which I'm not sure I understand yet: is is possible > to combine uprobes/kprobes decode more completely? It's not obvious > to me whether the uprobes-specific decoding only relates to features > which architecturally execute differently in user mode versus > privileged mode. Some explanation somewhere could be helpful. I just been looking at the decoding changes in patch 8 and had similar thoughts. The patch as it stands looks rather bolted on the side and makes the resulting code rather messy. My initial thoughts are that either: a) uprobes is similar enough to kprobes that the existing code can be morphed into something that cleanly supports both, or b) the similarities aren't close enough and that we should factor out the similarities into a more generalised decoding base, which the {u,k}probe code can then build on. c) some mix of a) and b) I can't help but think of the various calls over the past year or so for a general ARM/Thumb instruction decoding framework (the last one only a few weeks ago on the linux-arm-kernel list). Perhaps b) would be a small step towards that. I hope to find some time to understand the uprobe patches in more detail, so I can try and come up with some sensible suggestions on a cleaner solution; because I feel that as they stand they aren't really suitable for inclusion in the kernel. Rabin, what tree/commit are your patches based on? (They don't seem to apply cleanly to 3.6 or 3.7-rc1.) I want to apply them locally so I can use my favourite visualisation tool and to play with them. Thanks
On 10/14, Rabin Vincent wrote: > > @@ -655,6 +656,9 @@ do_work_pending(struct pt_regs *regs, unsigned int thread_flags, int syscall) > return restart; > } > syscall = 0; > + } else if (thread_flags & _TIF_UPROBE) { > + clear_thread_flag(TIF_UPROBE); > + uprobe_notify_resume(regs); > } else { > clear_thread_flag(TIF_NOTIFY_RESUME); > tracehook_notify_resume(regs); This doesn't look right. do_signal() can modify instruction pointer after we hit the breakpoint. IOW, uprobe_notify_resume() should be called before do_signal(). Oleg.
On Mon, Oct 15, 2012 at 06:31:47PM +0100, Dave Martin wrote: > On Sun, Oct 14, 2012 at 09:23:13PM +0200, Rabin Vincent wrote: > > +static int uprobe_trap_handler(struct pt_regs *regs, unsigned int instr) > > +{ > > + unsigned long flags; > > + > > + local_irq_save(flags); > > + if ((instr & 0x0fffffff) == UPROBE_SWBP_INSN) > > Is the check unnecessary here? I think the same comparison will > happen as a result of evaluating the associated undef_hook. The check is there because this uprobe_trap_handler() is registered for two different undefined instructions: UPROBE_SWBP_INSN (the one which is used to insert the probe) and UPROBE_SS_INSN (the one placed in the XOL area for simulating single-stepping).
On Wed, Oct 17, 2012 at 03:50:48PM +0100, Jon Medhurst (Tixy) wrote: > I just been looking at the decoding changes in patch 8 and had similar > thoughts. The patch as it stands looks rather bolted on the side and > makes the resulting code rather messy. I agree. > a) uprobes is similar enough to kprobes that the existing code can be > morphed into something that cleanly supports both, or > > b) the similarities aren't close enough and that we should factor out > the similarities into a more generalised decoding base, which the > {u,k}probe code can then build on. > > c) some mix of a) and b) > > I can't help but think of the various calls over the past year or so for > a general ARM/Thumb instruction decoding framework (the last one only a > few weeks ago on the linux-arm-kernel list). Perhaps b) would be a small > step towards that. > > I hope to find some time to understand the uprobe patches in more > detail, so I can try and come up with some sensible suggestions on a > cleaner solution; because I feel that as they stand they aren't really > suitable for inclusion in the kernel. I contemplated sending the decoding patch with [RFC] but finally went with [PATCH] since they mostly mean the same thing :-). Suggestions welcome. For one thing, the creation of a fake struct kprobe from within the uprobes and the dependency on kprobes because of that is not very nice, we probably need a "struct probe" of some sort perhaps. > Rabin, what tree/commit are your patches based on? (They don't seem to > apply cleanly to 3.6 or 3.7-rc1.) I want to apply them locally so I can > use my favourite visualisation tool and to play with them. The patches are based on next-20121012. The uprobes core is seeing quite a few changes in linux-next so the series will probably not apply on later linux-next trees.
On Mon, Oct 15, 2012 at 06:44:50PM +0100, Dave Martin wrote: > On Mon, Oct 15, 2012 at 01:44:55PM +0200, Rabin Vincent wrote: > > Why? It currently works for ARM userspace even if the kernel is > > Thumb-2. > > My bad, I misread what was happening in the Makefile changes. > > My concern is about whether we can build the ARM and Thumb-2 kprobes > code into the same kernel. If so, no problem, but I believe this is > not a tested configuration for kprobes itself. > > If you've not already done so, it should be possible to test this > by adding CONFIG_THUMB2_KERNEL=y to your config, providing your > hardware is Thumb-2 capable. I've tested this before. With a Thumb-2 kernel, both the kprobes test (Thumb) and the uprobes test (ARM) run fine. > General question which I'm not sure I understand yet: is is possible > to combine uprobes/kprobes decode more completely? It's not obvious > to me whether the uprobes-specific decoding only relates to features > which architecturally execute differently in user mode versus > privileged mode. Some explanation somewhere could be helpful. What we change is not the decoding itself but the handling of the instructions: (1) Load and stores are executed from the xol area by user space so the instructions need to be rewritten when they touch the PC. Kprobes code rewrites the instructions directly and executes them or in some cases simulates them. (2) All other non-simulated instructions are also executed from the XOL area in userspace. Because of this, the ALU instructions which use the PC also need to be rewritten to not use the PC. Perhaps we can actually get rid of this and just execute these instruction from kernel space like it is done for uprobes. So the uprobes code is uses the decoding tables just to know if the instruction is using the PC or not, but if we make the ALU instructions execute from kernel space we could actually use the emulate_*() functions like kprobes does.
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index 272c3a1..2191b61d 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -168,6 +168,10 @@ config ZONE_DMA config NEED_DMA_MAP_STATE def_bool y +config ARCH_SUPPORTS_UPROBES + depends on KPROBES + def_bool y + config ARCH_HAS_DMA_SET_COHERENT_MASK bool diff --git a/arch/arm/include/asm/ptrace.h b/arch/arm/include/asm/ptrace.h index 142d6ae..297936a 100644 --- a/arch/arm/include/asm/ptrace.h +++ b/arch/arm/include/asm/ptrace.h @@ -197,6 +197,12 @@ static inline long regs_return_value(struct pt_regs *regs) #define instruction_pointer(regs) (regs)->ARM_pc +static inline void instruction_pointer_set(struct pt_regs *regs, + unsigned long val) +{ + instruction_pointer(regs) = val; +} + #ifdef CONFIG_SMP extern unsigned long profile_pc(struct pt_regs *regs); #else diff --git a/arch/arm/include/asm/thread_info.h b/arch/arm/include/asm/thread_info.h index 8477b4c..7bedaee 100644 --- a/arch/arm/include/asm/thread_info.h +++ b/arch/arm/include/asm/thread_info.h @@ -148,6 +148,7 @@ extern int vfp_restore_user_hwstate(struct user_vfp __user *, #define TIF_SIGPENDING 0 #define TIF_NEED_RESCHED 1 #define TIF_NOTIFY_RESUME 2 /* callback before returning to user */ +#define TIF_UPROBE 7 #define TIF_SYSCALL_TRACE 8 #define TIF_SYSCALL_AUDIT 9 #define TIF_SYSCALL_TRACEPOINT 10 @@ -160,6 +161,7 @@ extern int vfp_restore_user_hwstate(struct user_vfp __user *, #define _TIF_SIGPENDING (1 << TIF_SIGPENDING) #define _TIF_NEED_RESCHED (1 << TIF_NEED_RESCHED) #define _TIF_NOTIFY_RESUME (1 << TIF_NOTIFY_RESUME) +#define _TIF_UPROBE (1 << TIF_UPROBE) #define _TIF_SYSCALL_TRACE (1 << TIF_SYSCALL_TRACE) #define _TIF_SYSCALL_AUDIT (1 << TIF_SYSCALL_AUDIT) #define _TIF_SYSCALL_TRACEPOINT (1 << TIF_SYSCALL_TRACEPOINT) @@ -172,7 +174,8 @@ extern int vfp_restore_user_hwstate(struct user_vfp __user *, /* * Change these and you break ASM code in entry-common.S */ -#define _TIF_WORK_MASK (_TIF_NEED_RESCHED | _TIF_SIGPENDING | _TIF_NOTIFY_RESUME) +#define _TIF_WORK_MASK (_TIF_NEED_RESCHED | _TIF_SIGPENDING | \ + _TIF_NOTIFY_RESUME | _TIF_UPROBE) #endif /* __KERNEL__ */ #endif /* __ASM_ARM_THREAD_INFO_H */ diff --git a/arch/arm/include/asm/uprobes.h b/arch/arm/include/asm/uprobes.h new file mode 100644 index 0000000..fa4b81e --- /dev/null +++ b/arch/arm/include/asm/uprobes.h @@ -0,0 +1,34 @@ +#ifndef _ASM_UPROBES_H +#define _ASM_UPROBES_H + +#include <asm/probes.h> + +typedef u32 uprobe_opcode_t; + +#define MAX_UINSN_BYTES 4 +#define UPROBE_XOL_SLOT_BYTES 64 + +#define UPROBE_SWBP_INSN 0x07f001f9 +#define UPROBE_SS_INSN 0x07f001fa +#define UPROBE_SWBP_INSN_SIZE 4 + +struct arch_uprobe_task { + u32 backup; +}; + +struct arch_uprobe { + u8 insn[MAX_UINSN_BYTES]; + uprobe_opcode_t modinsn; + uprobe_opcode_t bpinsn; + bool simulate; + u32 pcreg; + void (*prehandler)(struct arch_uprobe *auprobe, + struct arch_uprobe_task *autask, + struct pt_regs *regs); + void (*posthandler)(struct arch_uprobe *auprobe, + struct arch_uprobe_task *autask, + struct pt_regs *regs); + struct arch_specific_insn asi; +}; + +#endif diff --git a/arch/arm/kernel/Makefile b/arch/arm/kernel/Makefile index 5bbec7b..a39f634 100644 --- a/arch/arm/kernel/Makefile +++ b/arch/arm/kernel/Makefile @@ -40,6 +40,7 @@ obj-$(CONFIG_DYNAMIC_FTRACE) += ftrace.o insn.o obj-$(CONFIG_FUNCTION_GRAPH_TRACER) += ftrace.o insn.o obj-$(CONFIG_JUMP_LABEL) += jump_label.o insn.o patch.o obj-$(CONFIG_KEXEC) += machine_kexec.o relocate_kernel.o +obj-$(CONFIG_UPROBES) += uprobes.o uprobes-arm.o kprobes-arm.o obj-$(CONFIG_KPROBES) += kprobes.o kprobes-common.o patch.o ifdef CONFIG_THUMB2_KERNEL obj-$(CONFIG_KPROBES) += kprobes-thumb.o diff --git a/arch/arm/kernel/signal.c b/arch/arm/kernel/signal.c index 56f72d2..3b1e88e 100644 --- a/arch/arm/kernel/signal.c +++ b/arch/arm/kernel/signal.c @@ -12,6 +12,7 @@ #include <linux/personality.h> #include <linux/uaccess.h> #include <linux/tracehook.h> +#include <linux/uprobes.h> #include <asm/elf.h> #include <asm/cacheflush.h> @@ -655,6 +656,9 @@ do_work_pending(struct pt_regs *regs, unsigned int thread_flags, int syscall) return restart; } syscall = 0; + } else if (thread_flags & _TIF_UPROBE) { + clear_thread_flag(TIF_UPROBE); + uprobe_notify_resume(regs); } else { clear_thread_flag(TIF_NOTIFY_RESUME); tracehook_notify_resume(regs); diff --git a/arch/arm/kernel/uprobes.c b/arch/arm/kernel/uprobes.c new file mode 100644 index 0000000..f25a4af --- /dev/null +++ b/arch/arm/kernel/uprobes.c @@ -0,0 +1,191 @@ +/* + * Copyright (C) 2012 Rabin Vincent <rabin@rab.in> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +#include <linux/kernel.h> +#include <linux/sched.h> +#include <linux/uprobes.h> +#include <linux/notifier.h> +#include <linux/kprobes.h> + +#include <asm/opcodes.h> +#include <asm/traps.h> + +#include "kprobes.h" + +bool is_swbp_insn(uprobe_opcode_t *insn) +{ + return (__mem_to_opcode_arm(*insn) & 0x0fffffff) == UPROBE_SWBP_INSN; +} + +bool arch_uprobe_ignore(struct arch_uprobe *auprobe, struct pt_regs *regs) +{ + if (!auprobe->asi.insn_check_cc(regs->ARM_cpsr)) { + regs->ARM_pc += 4; + return true; + } + + return false; +} + +bool arch_uprobe_skip_sstep(struct arch_uprobe *auprobe, struct pt_regs *regs) +{ + struct kprobe kp; + + if (!auprobe->simulate) + return false; + + kp.addr = (void *) regs->ARM_pc; + kp.opcode = __mem_to_opcode_arm(*(unsigned int *) auprobe->insn); + kp.ainsn.insn_handler = auprobe->asi.insn_handler; + + auprobe->asi.insn_singlestep(&kp, regs); + + return true; +} + +int arch_uprobe_analyze_insn(struct arch_uprobe *auprobe, struct mm_struct *mm, + unsigned long addr) +{ + unsigned int insn; + unsigned int bpinsn; + enum kprobe_insn ret; + + /* Thumb not yet support */ + if (addr & 0x3) + return -EINVAL; + + insn = __mem_to_opcode_arm(*(unsigned int *)auprobe->insn); + auprobe->modinsn = insn; + + ret = arm_kprobe_decode_insn(insn, &auprobe->asi, true); + switch (ret) { + case INSN_REJECTED: + return -EINVAL; + + case INSN_GOOD_NO_SLOT: + auprobe->simulate = true; + break; + + case INSN_GOOD: + default: + break; + } + + bpinsn = UPROBE_SWBP_INSN; + if (insn >= 0xe0000000) + bpinsn |= 0xe0000000; /* Unconditional instruction */ + else + bpinsn |= insn & 0xf0000000; /* Copy condition from insn */ + + auprobe->bpinsn = bpinsn; + + return 0; +} + +void arch_uprobe_write_opcode(struct arch_uprobe *auprobe, void *vaddr, + uprobe_opcode_t opcode) +{ + unsigned long *addr = vaddr; + + if (opcode == UPROBE_SWBP_INSN) + opcode = __opcode_to_mem_arm(auprobe->bpinsn); + + *addr = opcode; +} + +void arch_uprobe_xol_copy(struct arch_uprobe *auprobe, void *vaddr) +{ + unsigned long *addr = vaddr; + + addr[0] = __opcode_to_mem_arm(auprobe->modinsn); + addr[1] = __opcode_to_mem_arm(0xe0000000 | UPROBE_SS_INSN); +} + +int arch_uprobe_pre_xol(struct arch_uprobe *auprobe, struct pt_regs *regs) +{ + struct uprobe_task *utask = current->utask; + + if (auprobe->prehandler) + auprobe->prehandler(auprobe, &utask->autask, regs); + + regs->ARM_pc = utask->xol_vaddr; + + return 0; +} + +int arch_uprobe_post_xol(struct arch_uprobe *auprobe, struct pt_regs *regs) +{ + struct uprobe_task *utask = current->utask; + + regs->ARM_pc = utask->vaddr + 4; + + if (auprobe->posthandler) + auprobe->posthandler(auprobe, &utask->autask, regs); + + return 0; +} + +bool arch_uprobe_xol_was_trapped(struct task_struct *t) +{ + /* TODO: implement */ + return false; +} + +void arch_uprobe_abort_xol(struct arch_uprobe *auprobe, struct pt_regs *regs) +{ + /* TODO: implement */ +} + +int arch_uprobe_exception_notify(struct notifier_block *self, + unsigned long val, void *data) +{ + return NOTIFY_DONE; +} + +static int uprobe_trap_handler(struct pt_regs *regs, unsigned int instr) +{ + unsigned long flags; + + local_irq_save(flags); + if ((instr & 0x0fffffff) == UPROBE_SWBP_INSN) + uprobe_pre_sstep_notifier(regs); + else + uprobe_post_sstep_notifier(regs); + local_irq_restore(flags); + + return 0; +} + +unsigned long uprobe_get_swbp_addr(struct pt_regs *regs) +{ + return instruction_pointer(regs); +} + +static struct undef_hook uprobes_arm_break_hook = { + .instr_mask = 0x0fffffff, + .instr_val = UPROBE_SWBP_INSN, + .cpsr_mask = MODE_MASK, + .cpsr_val = USR_MODE, + .fn = uprobe_trap_handler, +}; + +static struct undef_hook uprobes_arm_ss_hook = { + .instr_mask = 0x0fffffff, + .instr_val = UPROBE_SS_INSN, + .cpsr_mask = MODE_MASK, + .cpsr_val = USR_MODE, + .fn = uprobe_trap_handler, +}; + +int arch_uprobes_init(void) +{ + register_undef_hook(&uprobes_arm_break_hook); + register_undef_hook(&uprobes_arm_ss_hook); + + return 0; +}
Add basic uprobes support for ARM. perf probe --exec and SystemTap's userspace probing work. The ARM kprobes test code has also been run in a userspace harness to test the uprobe instruction decoding. Caveats: - Thumb is not supported - XOL abort/trap handling is not implemented Signed-off-by: Rabin Vincent <rabin@rab.in> --- arch/arm/Kconfig | 4 + arch/arm/include/asm/ptrace.h | 6 ++ arch/arm/include/asm/thread_info.h | 5 +- arch/arm/include/asm/uprobes.h | 34 +++++++ arch/arm/kernel/Makefile | 1 + arch/arm/kernel/signal.c | 4 + arch/arm/kernel/uprobes.c | 191 ++++++++++++++++++++++++++++++++++++ 7 files changed, 244 insertions(+), 1 deletion(-) create mode 100644 arch/arm/include/asm/uprobes.h create mode 100644 arch/arm/kernel/uprobes.c