'fed240d9c974 ("ARM: Recover kretprobe modified return address in stacktrace")'
has implemented kretprobes return address recovery for FP
unwinder, this patch makes it works for EABI unwinder.
It saves __kretprobe_trampoline address in LR on stack to identify
and recover the correct return address in EABI unwinder.
Since EABI doesn't use r11 as frame pointer, we need to use SP to
identify different kretprobes addresses. Here the value of SP has fixed
distance to conventional FP position so it's fine to use it.
Passed kunit kprobes_test on QEMU.
Signed-off-by: Chen Zhongjin <[email protected]>
---
arch/arm/Kconfig | 2 +-
arch/arm/kernel/unwind.c | 12 ++++++++++++
arch/arm/probes/kprobes/core.c | 20 +++++++++++++++++---
3 files changed, 30 insertions(+), 4 deletions(-)
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 87badeae3181..7165e314c0de 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -3,7 +3,7 @@ config ARM
bool
default y
select ARCH_32BIT_OFF_T
- select ARCH_CORRECT_STACKTRACE_ON_KRETPROBE if HAVE_KRETPROBES && FRAME_POINTER && !ARM_UNWIND
+ select ARCH_CORRECT_STACKTRACE_ON_KRETPROBE if HAVE_KRETPROBES
select ARCH_HAS_BINFMT_FLAT
select ARCH_HAS_CURRENT_STACK_POINTER
select ARCH_HAS_DEBUG_VIRTUAL if MMU
diff --git a/arch/arm/kernel/unwind.c b/arch/arm/kernel/unwind.c
index a37ea6c772cd..51e34fa4a4b3 100644
--- a/arch/arm/kernel/unwind.c
+++ b/arch/arm/kernel/unwind.c
@@ -28,6 +28,7 @@
#include <linux/slab.h>
#include <linux/spinlock.h>
#include <linux/list.h>
+#include <linux/kprobes.h>
#include <asm/stacktrace.h>
#include <asm/traps.h>
@@ -482,6 +483,12 @@ int unwind_frame(struct stackframe *frame)
frame->pc = ctrl.vrs[PC];
frame->lr_addr = ctrl.lr_addr;
+#ifdef CONFIG_KRETPROBES
+ if (is_kretprobe_trampoline(frame->pc))
+ frame->pc = kretprobe_find_ret_addr(frame->tsk,
+ (void *)frame->sp, &frame->kr_cur);
+#endif
+
return URC_OK;
}
@@ -522,6 +529,11 @@ void unwind_backtrace(struct pt_regs *regs, struct task_struct *tsk,
frame.pc = thread_saved_pc(tsk);
}
+#ifdef CONFIG_KRETPROBES
+ frame.kr_cur = NULL;
+ frame.tsk = tsk;
+#endif
+
while (1) {
int urc;
unsigned long where = frame.pc;
diff --git a/arch/arm/probes/kprobes/core.c b/arch/arm/probes/kprobes/core.c
index 9090c3a74dcc..1435b508aa36 100644
--- a/arch/arm/probes/kprobes/core.c
+++ b/arch/arm/probes/kprobes/core.c
@@ -41,6 +41,16 @@
(unsigned long)(addr) + \
(size))
+/*
+ * Since EABI unwinder doesn't use ARM_fp as conventional fp
+ * use ARM_sp as hint register for kretprobes.
+ */
+#ifdef CONFIG_ARM_UNWIND
+#define TRAMP_FP ARM_sp
+#else /* CONFIG_FRAME_POINTER */
+#define TRAMP_FP ARM_fp
+#endif
+
DEFINE_PER_CPU(struct kprobe *, current_kprobe) = NULL;
DEFINE_PER_CPU(struct kprobe_ctlblk, kprobe_ctlblk);
@@ -376,8 +386,8 @@ int __kprobes kprobe_exceptions_notify(struct notifier_block *self,
void __naked __kprobes __kretprobe_trampoline(void)
{
__asm__ __volatile__ (
-#ifdef CONFIG_FRAME_POINTER
"ldr lr, =__kretprobe_trampoline \n\t"
+#ifdef CONFIG_FRAME_POINTER
/* __kretprobe_trampoline makes a framepointer on pt_regs. */
#ifdef CONFIG_CC_IS_CLANG
"stmdb sp, {sp, lr, pc} \n\t"
@@ -395,8 +405,12 @@ void __naked __kprobes __kretprobe_trampoline(void)
"add fp, sp, #60 \n\t"
#endif /* CONFIG_CC_IS_CLANG */
#else /* !CONFIG_FRAME_POINTER */
+ /* store SP, LR on stack and add EABI unwind hint */
+ "stmdb sp, {sp, lr, pc} \n\t"
+ ".save {sp, lr, pc} \n\t"
"sub sp, sp, #16 \n\t"
"stmdb sp!, {r0 - r11} \n\t"
+ ".pad #52 \n\t"
#endif /* CONFIG_FRAME_POINTER */
"mov r0, sp \n\t"
"bl trampoline_handler \n\t"
@@ -414,14 +428,14 @@ void __naked __kprobes __kretprobe_trampoline(void)
/* Called from __kretprobe_trampoline */
static __used __kprobes void *trampoline_handler(struct pt_regs *regs)
{
- return (void *)kretprobe_trampoline_handler(regs, (void *)regs->ARM_fp);
+ return (void *)kretprobe_trampoline_handler(regs, (void *)regs->TRAMP_FP);
}
void __kprobes arch_prepare_kretprobe(struct kretprobe_instance *ri,
struct pt_regs *regs)
{
ri->ret_addr = (kprobe_opcode_t *)regs->ARM_lr;
- ri->fp = (void *)regs->ARM_fp;
+ ri->fp = (void *)regs->TRAMP_FP;
/* Replace the return addr with trampoline addr. */
regs->ARM_lr = (unsigned long)&__kretprobe_trampoline;
--
2.17.1
On Mon, Aug 15, 2022 at 1:06 PM Chen Zhongjin <[email protected]> wrote:
> 'fed240d9c974 ("ARM: Recover kretprobe modified return address in stacktrace")'
> has implemented kretprobes return address recovery for FP
> unwinder, this patch makes it works for EABI unwinder.
>
> It saves __kretprobe_trampoline address in LR on stack to identify
> and recover the correct return address in EABI unwinder.
>
> Since EABI doesn't use r11 as frame pointer, we need to use SP to
> identify different kretprobes addresses. Here the value of SP has fixed
> distance to conventional FP position so it's fine to use it.
>
> Passed kunit kprobes_test on QEMU.
>
> Signed-off-by: Chen Zhongjin <[email protected]>
This looks correct to me FWIW I understand the assembly
but I'm vaguely unfamiliar with the kprobe API, but anyway:
Reviewed-by: Linus Walleij <[email protected]>
Would you please drop it into Russell's patch tracker?
Yours,
Linus Walleij
On 2022/8/22 19:20, Linus Walleij wrote:
> On Mon, Aug 15, 2022 at 1:06 PM Chen Zhongjin <[email protected]> wrote:
>
>> 'fed240d9c974 ("ARM: Recover kretprobe modified return address in stacktrace")'
>> has implemented kretprobes return address recovery for FP
>> unwinder, this patch makes it works for EABI unwinder.
>>
>> It saves __kretprobe_trampoline address in LR on stack to identify
>> and recover the correct return address in EABI unwinder.
>>
>> Since EABI doesn't use r11 as frame pointer, we need to use SP to
>> identify different kretprobes addresses. Here the value of SP has fixed
>> distance to conventional FP position so it's fine to use it.
>>
>> Passed kunit kprobes_test on QEMU.
>>
>> Signed-off-by: Chen Zhongjin <[email protected]>
> This looks correct to me FWIW I understand the assembly
> but I'm vaguely unfamiliar with the kprobe API, but anyway:
> Reviewed-by: Linus Walleij <[email protected]>
>
> Would you please drop it into Russell's patch tracker?
Done for it.
Thanks for review!
Best,
Chen
> Yours,
> Linus Walleij