From: Marc Zyngier <marc.zyngier@arm.com>
To: David Long <dave.long@linaro.org>,
Catalin Marinas <catalin.marinas@arm.com>,
Will Deacon <will.deacon@arm.com>,
Sandeepa Prabhu <sandeepa.s.prabhu@gmail.com>,
William Cohen <wcohen@redhat.com>,
Pratyush Anand <panand@redhat.com>,
Steve Capper <steve.capper@linaro.org>,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
Cc: "Dave P Martin" <Dave.Martin@arm.com>,
"Mark Rutland" <mark.rutland@arm.com>,
"Robin Murphy" <Robin.Murphy@arm.com>,
"Ard Biesheuvel" <ard.biesheuvel@linaro.org>,
"Jens Wiklander" <jens.wiklander@linaro.org>,
"Christoffer Dall" <christoffer.dall@linaro.org>,
"Alex Bennée" <alex.bennee@linaro.org>,
"Yang Shi" <yang.shi@linaro.org>,
"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
"Viresh Kumar" <viresh.kumar@linaro.org>,
"Suzuki K. Poulose" <suzuki.poulose@arm.com>,
"Kees Cook" <keescook@chromium.org>,
"Zi Shen Lim" <zlim.lnx@gmail.com>,
"John Blackwood" <john.blackwood@ccur.com>,
"Feng Kan" <fkan@apm.com>,
"Balamurugan Shanmugam" <bshanmugam@apm.com>,
"James Morse" <james.morse@arm.com>,
"Vladimir Murzin" <Vladimir.Murzin@arm.com>,
"Mark Salyzyn" <salyzyn@android.com>,
"Petr Mladek" <pmladek@suse.com>,
"Andrew Morton" <akpm@linux-foundation.org>,
"Mark Brown" <broonie@kernel.org>
Subject: Re: [PATCH v10 7/9] arm64: Add trampoline code for kretprobes
Date: Tue, 1 Mar 2016 18:19:19 +0000 [thread overview]
Message-ID: <56D5DD27.6040404@arm.com> (raw)
In-Reply-To: <1456801047-29014-8-git-send-email-dave.long@linaro.org>
On 01/03/16 02:57, David Long wrote:
> From: William Cohen <wcohen@redhat.com>
>
> The trampoline code is used by kretprobes to capture a return from a probed
> function. This is done by saving the registers, calling the handler, and
> restoring the registers. The code then returns to the original saved caller
> return address. It is necessary to do this directly instead of using a
> software breakpoint because the code used in processing that breakpoint
> could itself be kprobe'd and cause a problematic reentry into the debug
> exception handler.
>
> Signed-off-by: William Cohen <wcohen@redhat.com>
> Signed-off-by: David A. Long <dave.long@linaro.org>
> ---
> arch/arm64/include/asm/kprobes.h | 2 +
> arch/arm64/kernel/Makefile | 1 +
> arch/arm64/kernel/asm-offsets.c | 22 +++++++++++
> arch/arm64/kernel/kprobes.c | 5 +++
> arch/arm64/kernel/kprobes_trampoline.S | 67 ++++++++++++++++++++++++++++++++++
> 5 files changed, 97 insertions(+)
> create mode 100644 arch/arm64/kernel/kprobes_trampoline.S
>
> diff --git a/arch/arm64/include/asm/kprobes.h b/arch/arm64/include/asm/kprobes.h
> index 79c9511..61b4915 100644
> --- a/arch/arm64/include/asm/kprobes.h
> +++ b/arch/arm64/include/asm/kprobes.h
> @@ -56,5 +56,7 @@ int kprobe_exceptions_notify(struct notifier_block *self,
> unsigned long val, void *data);
> int kprobe_breakpoint_handler(struct pt_regs *regs, unsigned int esr);
> int kprobe_single_step_handler(struct pt_regs *regs, unsigned int esr);
> +void kretprobe_trampoline(void);
> +void __kprobes *trampoline_probe_handler(struct pt_regs *regs);
>
> #endif /* _ARM_KPROBES_H */
> diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
> index 08325e5..f192b7d 100644
> --- a/arch/arm64/kernel/Makefile
> +++ b/arch/arm64/kernel/Makefile
> @@ -37,6 +37,7 @@ arm64-obj-$(CONFIG_CPU_IDLE) += cpuidle.o
> arm64-obj-$(CONFIG_JUMP_LABEL) += jump_label.o
> arm64-obj-$(CONFIG_KGDB) += kgdb.o
> arm64-obj-$(CONFIG_KPROBES) += kprobes.o kprobes-arm64.o \
> + kprobes_trampoline.o \
> probes-simulate-insn.o
> arm64-obj-$(CONFIG_EFI) += efi.o efi-entry.stub.o
> arm64-obj-$(CONFIG_PCI) += pci.o
> diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c
> index fffa4ac6..460b54c 100644
> --- a/arch/arm64/kernel/asm-offsets.c
> +++ b/arch/arm64/kernel/asm-offsets.c
> @@ -50,6 +50,28 @@ int main(void)
> DEFINE(S_X5, offsetof(struct pt_regs, regs[5]));
> DEFINE(S_X6, offsetof(struct pt_regs, regs[6]));
> DEFINE(S_X7, offsetof(struct pt_regs, regs[7]));
> + DEFINE(S_X8, offsetof(struct pt_regs, regs[8]));
> + DEFINE(S_X9, offsetof(struct pt_regs, regs[9]));
> + DEFINE(S_X10, offsetof(struct pt_regs, regs[10]));
> + DEFINE(S_X11, offsetof(struct pt_regs, regs[11]));
> + DEFINE(S_X12, offsetof(struct pt_regs, regs[12]));
> + DEFINE(S_X13, offsetof(struct pt_regs, regs[13]));
> + DEFINE(S_X14, offsetof(struct pt_regs, regs[14]));
> + DEFINE(S_X15, offsetof(struct pt_regs, regs[15]));
> + DEFINE(S_X16, offsetof(struct pt_regs, regs[16]));
> + DEFINE(S_X17, offsetof(struct pt_regs, regs[17]));
> + DEFINE(S_X18, offsetof(struct pt_regs, regs[18]));
> + DEFINE(S_X19, offsetof(struct pt_regs, regs[19]));
> + DEFINE(S_X20, offsetof(struct pt_regs, regs[20]));
> + DEFINE(S_X21, offsetof(struct pt_regs, regs[21]));
> + DEFINE(S_X22, offsetof(struct pt_regs, regs[22]));
> + DEFINE(S_X23, offsetof(struct pt_regs, regs[23]));
> + DEFINE(S_X24, offsetof(struct pt_regs, regs[24]));
> + DEFINE(S_X25, offsetof(struct pt_regs, regs[25]));
> + DEFINE(S_X26, offsetof(struct pt_regs, regs[26]));
> + DEFINE(S_X27, offsetof(struct pt_regs, regs[27]));
> + DEFINE(S_X28, offsetof(struct pt_regs, regs[28]));
> + DEFINE(S_X29, offsetof(struct pt_regs, regs[29]));
Do we need all of these? Specially considering that we're only using the
even ones? You may want to consider something like
arch/arm64/kvm/hyp/entry.S does.
> DEFINE(S_LR, offsetof(struct pt_regs, regs[30]));
> DEFINE(S_SP, offsetof(struct pt_regs, sp));
> #ifdef CONFIG_COMPAT
> diff --git a/arch/arm64/kernel/kprobes.c b/arch/arm64/kernel/kprobes.c
> index ffc5affd..98f4fe5 100644
> --- a/arch/arm64/kernel/kprobes.c
> +++ b/arch/arm64/kernel/kprobes.c
> @@ -532,6 +532,11 @@ int __kprobes longjmp_break_handler(struct kprobe *p, struct pt_regs *regs)
> return 1;
> }
>
> +void __kprobes __used *trampoline_probe_handler(struct pt_regs *regs)
> +{
> + return (void *) 0;
Something wrong with NULL? ;-)
> +}
> +
> int __init arch_init_kprobes(void)
> {
> return 0;
> diff --git a/arch/arm64/kernel/kprobes_trampoline.S b/arch/arm64/kernel/kprobes_trampoline.S
> new file mode 100644
> index 0000000..5a336cf
> --- /dev/null
> +++ b/arch/arm64/kernel/kprobes_trampoline.S
> @@ -0,0 +1,67 @@
> +/*
> + * trampoline entry and return code for kretprobes.
> + */
> +
> +#include <linux/linkage.h>
> +#include <generated/asm-offsets.h>
#include <asm/asm-offsets.h>
> +
> + .text
> +
> +ENTRY(kretprobe_trampoline)
> +
> + sub sp, sp, #S_FRAME_SIZE
> +
> + stp x0, x1, [sp, #S_X0]
> + stp x2, x3, [sp, #S_X2]
> + stp x4, x5, [sp, #S_X4]
> + stp x6, x7, [sp, #S_X6]
> + stp x8, x9, [sp, #S_X8]
> + stp x10, x11, [sp, #S_X10]
> + stp x12, x13, [sp, #S_X12]
> + stp x14, x15, [sp, #S_X14]
> + stp x16, x17, [sp, #S_X16]
> + stp x18, x19, [sp, #S_X18]
> + stp x20, x21, [sp, #S_X20]
> + stp x22, x23, [sp, #S_X22]
> + stp x24, x25, [sp, #S_X24]
> + stp x26, x27, [sp, #S_X26]
> + stp x28, x29, [sp, #S_X28]
> + str x30, [sp, #S_LR]
Might as well call it LR?
> + add x0, sp, #S_FRAME_SIZE
> + str x0, [sp, #S_SP]
> + mrs x0, nzcv
> + mrs x1, daif
> + orr x0, x0, x1
> + /* There seems no easy way to get the mode field so make one up */
> + add x0, x0, #5
Do you mean something like CurrentEL? You could also save SPSel whilst
you're at it.
> + str x0, [sp, #S_PSTATE]
> +
> + mov x0, sp
> + bl trampoline_probe_handler
> + /* Replace trampoline address in lr with actual
> + orig_ret_addr return address. */
> + str x0, [sp, #S_LR]
Why do you need to store it on the stack? You could do a "mov lr, x0",
and drop the last load of the sequence below...
> +
> + ldr x0, [sp, #S_PSTATE]
> + msr nzcv, x0
> + ldp x0, x1, [sp, #S_X0]
> + ldp x2, x3, [sp, #S_X2]
> + ldp x4, x5, [sp, #S_X4]
> + ldp x6, x7, [sp, #S_X6]
> + ldp x8, x9, [sp, #S_X8]
> + ldp x10, x11, [sp, #S_X10]
> + ldp x12, x13, [sp, #S_X12]
> + ldp x14, x15, [sp, #S_X14]
> + ldp x16, x17, [sp, #S_X16]
> + ldp x18, x19, [sp, #S_X18]
> + ldp x20, x21, [sp, #S_X20]
> + ldp x22, x23, [sp, #S_X22]
> + ldp x24, x25, [sp, #S_X24]
> + ldp x26, x27, [sp, #S_X26]
> + ldp x28, x29, [sp, #S_X28]
> + ldr x30, [sp, #S_LR]
> +
> + add sp, sp, #S_FRAME_SIZE
> + ret
> +
> +ENDPROC(kretprobe_trampoline)
>
Thanks,
M.
--
Jazz is not dead. It just smells funny...
next prev parent reply other threads:[~2016-03-01 18:19 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-03-01 2:57 [PATCH v10 0/9] arm64: Add kernel probes (kprobes) support David Long
2016-03-01 2:57 ` [PATCH v10 1/9] arm64: Add HAVE_REGS_AND_STACK_ACCESS_API feature David Long
2016-03-14 9:41 ` 平松雅巳 / HIRAMATU，MASAMI
2016-03-01 2:57 ` [PATCH v10 2/9] arm64: Add more test functions to insn.c David Long
2016-03-01 2:57 ` [PATCH v10 3/9] arm64: add copy_to/from_user to kprobes blacklist David Long
2016-03-01 2:57 ` [PATCH v10 4/9] arm64: add conditional instruction simulation support David Long
2016-03-01 17:43 ` Marc Zyngier
2016-03-02 5:14 ` David Long
2016-03-01 2:57 ` [PATCH v10 5/9] arm64: Kprobes with single stepping support David Long
2016-03-01 2:57 ` [PATCH v10 6/9] arm64: kprobes instruction simulation support David Long
2016-03-01 18:04 ` Marc Zyngier
2016-03-03 5:02 ` David Long
2016-03-03 8:01 ` Marc Zyngier
2016-03-03 15:14 ` David Long
2016-03-03 15:32 ` Marc Zyngier
2016-03-01 2:57 ` [PATCH v10 7/9] arm64: Add trampoline code for kretprobes David Long
2016-03-01 18:19 ` Marc Zyngier [this message]
2016-03-02 21:20 ` William Cohen
2016-03-08 5:42 ` David Long
2016-03-01 2:57 ` [PATCH v10 8/9] arm64: Add kernel return probes support (kretprobes) David Long
2016-03-01 2:57 ` [PATCH v10 9/9] kprobes: Add arm64 case in kprobe example module David Long
2016-03-14 9:36 ` [PATCH v10 0/9] arm64: Add kernel probes (kprobes) support 平松雅巳 / HIRAMATU，MASAMI
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=56D5DD27.6040404@arm.com \
--to=marc.zyngier@arm.com \
--cc=Dave.Martin@arm.com \
--cc=Robin.Murphy@arm.com \
--cc=Vladimir.Murzin@arm.com \
--cc=akpm@linux-foundation.org \
--cc=alex.bennee@linaro.org \
--cc=ard.biesheuvel@linaro.org \
--cc=broonie@kernel.org \
--cc=bshanmugam@apm.com \
--cc=catalin.marinas@arm.com \
--cc=christoffer.dall@linaro.org \
--cc=dave.long@linaro.org \
--cc=fkan@apm.com \
--cc=gregkh@linuxfoundation.org \
--cc=james.morse@arm.com \
--cc=jens.wiklander@linaro.org \
--cc=john.blackwood@ccur.com \
--cc=keescook@chromium.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=panand@redhat.com \
--cc=pmladek@suse.com \
--cc=salyzyn@android.com \
--cc=sandeepa.s.prabhu@gmail.com \
--cc=steve.capper@linaro.org \
--cc=suzuki.poulose@arm.com \
--cc=viresh.kumar@linaro.org \
--cc=wcohen@redhat.com \
--cc=will.deacon@arm.com \
--cc=yang.shi@linaro.org \
--cc=zlim.lnx@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox