When unwinding on ftrace_regs_call, the traced function will be skipped
because ftrace_regs_caller doesn't save the fp and ra.
Save the encoded fp so that we can get the pt_regs so that we can unwind
from ftrace_regs_call to the traced function.
Also the pt_regs->status should be set as kernel mode.
Stacktrace before this patch:
Call Trace:
...
[<ffffffff0161a0e0>] handler_pre+0x30/0x4a [kprobe_unwind]
[<ffffffff800bce92>] aggr_pre_handler+0x60/0x94
[<ffffffff80006df0>] kprobe_ftrace_handler+0x13e/0x188
[<ffffffff80008e7e>] ftrace_regs_call+0x8/0x10
[<ffffffff80002540>] do_one_initcall+0x4c/0x1f2
[<ffffffff8008a4e6>] do_init_module+0x56/0x210
...
Stacktrace after this patch:
Call Trace:
...
[<ffffffff016150e0>] handler_pre+0x30/0x4a [kprobe_unwind]
[<ffffffff800bce96>] aggr_pre_handler+0x60/0x94
[<ffffffff80006df0>] kprobe_ftrace_handler+0x13e/0x188
[<ffffffff80008e82>] ftrace_regs_call+0x8/0x10
+ [<ffffffff01615000>] empty_call+0x0/0x1e [kprobe_unwind]
[<ffffffff80002540>] do_one_initcall+0x4c/0x1f2
[<ffffffff8008a4ea>] do_init_module+0x56/0x210
...
Signed-off-by: Chen Zhongjin <[email protected]>
---
arch/riscv/kernel/mcount-dyn.S | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/arch/riscv/kernel/mcount-dyn.S b/arch/riscv/kernel/mcount-dyn.S
index d171eca623b6..56a4014c392f 100644
--- a/arch/riscv/kernel/mcount-dyn.S
+++ b/arch/riscv/kernel/mcount-dyn.S
@@ -10,6 +10,8 @@
#include <asm/asm-offsets.h>
#include <asm-generic/export.h>
#include <asm/ftrace.h>
+#include <asm/frame.h>
+#include <asm/csr.h>
.text
@@ -97,6 +99,11 @@
REG_S x29, PT_T4(sp)
REG_S x30, PT_T5(sp)
REG_S x31, PT_T6(sp)
+
+#ifdef CONFIG_FRAME_POINTER
+ li s0, SR_PP
+ REG_S s0, PT_STATUS(sp)
+#endif
.endm
.macro RESTORE_ALL
@@ -172,6 +179,7 @@ ENDPROC(ftrace_caller)
#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
ENTRY(ftrace_regs_caller)
SAVE_ALL
+ ENCODE_FRAME_POINTER
addi a0, ra, -FENTRY_RA_OFFSET
la a1, function_trace_op
--
2.17.1
On Tue, Sep 20, 2022 at 11:15 PM Chen Zhongjin <[email protected]> wrote:
>
> When unwinding on ftrace_regs_call, the traced function will be skipped
> because ftrace_regs_caller doesn't save the fp and ra.
>
> Save the encoded fp so that we can get the pt_regs so that we can unwind
> from ftrace_regs_call to the traced function.
>
> Also the pt_regs->status should be set as kernel mode.
>
> Stacktrace before this patch:
>
> Call Trace:
> ...
> [<ffffffff0161a0e0>] handler_pre+0x30/0x4a [kprobe_unwind]
> [<ffffffff800bce92>] aggr_pre_handler+0x60/0x94
> [<ffffffff80006df0>] kprobe_ftrace_handler+0x13e/0x188
> [<ffffffff80008e7e>] ftrace_regs_call+0x8/0x10
> [<ffffffff80002540>] do_one_initcall+0x4c/0x1f2
> [<ffffffff8008a4e6>] do_init_module+0x56/0x210
> ...
>
> Stacktrace after this patch:
>
> Call Trace:
> ...
> [<ffffffff016150e0>] handler_pre+0x30/0x4a [kprobe_unwind]
> [<ffffffff800bce96>] aggr_pre_handler+0x60/0x94
> [<ffffffff80006df0>] kprobe_ftrace_handler+0x13e/0x188
> [<ffffffff80008e82>] ftrace_regs_call+0x8/0x10
> + [<ffffffff01615000>] empty_call+0x0/0x1e [kprobe_unwind]
> [<ffffffff80002540>] do_one_initcall+0x4c/0x1f2
> [<ffffffff8008a4ea>] do_init_module+0x56/0x210
> ...
>
> Signed-off-by: Chen Zhongjin <[email protected]>
> ---
> arch/riscv/kernel/mcount-dyn.S | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/arch/riscv/kernel/mcount-dyn.S b/arch/riscv/kernel/mcount-dyn.S
> index d171eca623b6..56a4014c392f 100644
> --- a/arch/riscv/kernel/mcount-dyn.S
> +++ b/arch/riscv/kernel/mcount-dyn.S
> @@ -10,6 +10,8 @@
> #include <asm/asm-offsets.h>
> #include <asm-generic/export.h>
> #include <asm/ftrace.h>
> +#include <asm/frame.h>
> +#include <asm/csr.h>
>
> .text
>
> @@ -97,6 +99,11 @@
> REG_S x29, PT_T4(sp)
> REG_S x30, PT_T5(sp)
> REG_S x31, PT_T6(sp)
> +
> +#ifdef CONFIG_FRAME_POINTER
> + li s0, SR_PP
> + REG_S s0, PT_STATUS(sp)
Change another register.
> +#endif
> .endm
>
> .macro RESTORE_ALL
> @@ -172,6 +179,7 @@ ENDPROC(ftrace_caller)
> #ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
> ENTRY(ftrace_regs_caller)
> SAVE_ALL
> + ENCODE_FRAME_POINTER
Maybe the patch should merge with others, so separated make it hard to review.
>
> addi a0, ra, -FENTRY_RA_OFFSET
> la a1, function_trace_op
> --
> 2.17.1
>
--
Best Regards
Guo Ren
Hi,
On 2022/9/21 9:43, Guo Ren wrote:
> On Tue, Sep 20, 2022 at 11:15 PM Chen Zhongjin <[email protected]> wrote:
>> When unwinding on ftrace_regs_call, the traced function will be skipped
>> because ftrace_regs_caller doesn't save the fp and ra.
>>
>> Save the encoded fp so that we can get the pt_regs so that we can unwind
>> from ftrace_regs_call to the traced function.
>>
>> Also the pt_regs->status should be set as kernel mode.
>>
>> Stacktrace before this patch:
>>
>> Call Trace:
>> ...
>> [<ffffffff0161a0e0>] handler_pre+0x30/0x4a [kprobe_unwind]
>> [<ffffffff800bce92>] aggr_pre_handler+0x60/0x94
>> [<ffffffff80006df0>] kprobe_ftrace_handler+0x13e/0x188
>> [<ffffffff80008e7e>] ftrace_regs_call+0x8/0x10
>> [<ffffffff80002540>] do_one_initcall+0x4c/0x1f2
>> [<ffffffff8008a4e6>] do_init_module+0x56/0x210
>> ...
>>
>> Stacktrace after this patch:
>>
>> Call Trace:
>> ...
>> [<ffffffff016150e0>] handler_pre+0x30/0x4a [kprobe_unwind]
>> [<ffffffff800bce96>] aggr_pre_handler+0x60/0x94
>> [<ffffffff80006df0>] kprobe_ftrace_handler+0x13e/0x188
>> [<ffffffff80008e82>] ftrace_regs_call+0x8/0x10
>> + [<ffffffff01615000>] empty_call+0x0/0x1e [kprobe_unwind]
>> [<ffffffff80002540>] do_one_initcall+0x4c/0x1f2
>> [<ffffffff8008a4ea>] do_init_module+0x56/0x210
>> ...
>>
>> Signed-off-by: Chen Zhongjin <[email protected]>
>> ---
>> arch/riscv/kernel/mcount-dyn.S | 8 ++++++++
>> 1 file changed, 8 insertions(+)
>>
>> diff --git a/arch/riscv/kernel/mcount-dyn.S b/arch/riscv/kernel/mcount-dyn.S
>> index d171eca623b6..56a4014c392f 100644
>> --- a/arch/riscv/kernel/mcount-dyn.S
>> +++ b/arch/riscv/kernel/mcount-dyn.S
>> @@ -10,6 +10,8 @@
>> #include <asm/asm-offsets.h>
>> #include <asm-generic/export.h>
>> #include <asm/ftrace.h>
>> +#include <asm/frame.h>
>> +#include <asm/csr.h>
>>
>> .text
>>
>> @@ -97,6 +99,11 @@
>> REG_S x29, PT_T4(sp)
>> REG_S x30, PT_T5(sp)
>> REG_S x31, PT_T6(sp)
>> +
>> +#ifdef CONFIG_FRAME_POINTER
>> + li s0, SR_PP
>> + REG_S s0, PT_STATUS(sp)
> Change another register.
>
>> +#endif
>> .endm
>>
>> .macro RESTORE_ALL
>> @@ -172,6 +179,7 @@ ENDPROC(ftrace_caller)
>> #ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
>> ENTRY(ftrace_regs_caller)
>> SAVE_ALL
>> + ENCODE_FRAME_POINTER
> Maybe the patch should merge with others, so separated make it hard to review.
Thanks for review!
Happy to see you have approved the ENCODE FP part.
The patches is separated just because I wanted to paste the call trace
result in commit message.
I'll merge the ENCODE_FRAME_POINTER patches all together and do this in
the cover.
Also noticed another email for your ongoing patches. I'll review them
and reply later.
Best,
Chen