* [PATCH v1 2/4] powerpc/ftrace: Add recursion protection in prepare_ftrace_return()
2022-02-15 18:31 [PATCH v1 1/4] powerpc/ftrace: Also save r1 in ftrace_caller() Christophe Leroy
@ 2022-02-15 18:31 ` Christophe Leroy
2022-02-15 18:31 ` [PATCH v1 3/4] powerpc/ftrace: Have arch_ftrace_get_regs() return NULL unless FL_SAVE_REGS is set Christophe Leroy
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Christophe Leroy @ 2022-02-15 18:31 UTC (permalink / raw)
To: Steven Rostedt, Ingo Molnar, Benjamin Herrenschmidt,
Paul Mackerras, Michael Ellerman
Cc: Naveen N . Rao, linuxppc-dev, linux-kernel
The function_graph_enter() does not provide any recursion protection.
Add a protection in prepare_ftrace_return() in case
function_graph_enter() calls something that gets
function graph traced.
Reported-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
Fixes: 830213786c49 ("powerpc/ftrace: directly call of function graph tracer by ftrace caller")
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/kernel/trace/ftrace.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/arch/powerpc/kernel/trace/ftrace.c b/arch/powerpc/kernel/trace/ftrace.c
index 74a176e394ef..f21b8fbd418e 100644
--- a/arch/powerpc/kernel/trace/ftrace.c
+++ b/arch/powerpc/kernel/trace/ftrace.c
@@ -944,6 +944,7 @@ unsigned long prepare_ftrace_return(unsigned long parent, unsigned long ip,
unsigned long sp)
{
unsigned long return_hooker;
+ int bit;
if (unlikely(ftrace_graph_is_dead()))
goto out;
@@ -951,10 +952,16 @@ unsigned long prepare_ftrace_return(unsigned long parent, unsigned long ip,
if (unlikely(atomic_read(¤t->tracing_graph_pause)))
goto out;
+ bit = ftrace_test_recursion_trylock(ip, parent);
+ if (bit < 0)
+ goto out;
+
return_hooker = ppc_function_entry(return_to_handler);
if (!function_graph_enter(parent, ip, 0, (unsigned long *)sp))
parent = return_hooker;
+
+ ftrace_test_recursion_unlock(bit);
out:
return parent;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v1 3/4] powerpc/ftrace: Have arch_ftrace_get_regs() return NULL unless FL_SAVE_REGS is set
2022-02-15 18:31 [PATCH v1 1/4] powerpc/ftrace: Also save r1 in ftrace_caller() Christophe Leroy
2022-02-15 18:31 ` [PATCH v1 2/4] powerpc/ftrace: Add recursion protection in prepare_ftrace_return() Christophe Leroy
@ 2022-02-15 18:31 ` Christophe Leroy
2022-02-15 18:31 ` [PATCH v1 4/4] powerpc/ftrace: Style cleanup in ftrace_mprofile.S Christophe Leroy
2022-03-02 12:41 ` [PATCH v1 1/4] powerpc/ftrace: Also save r1 in ftrace_caller() Michael Ellerman
3 siblings, 0 replies; 6+ messages in thread
From: Christophe Leroy @ 2022-02-15 18:31 UTC (permalink / raw)
To: Steven Rostedt, Ingo Molnar, Benjamin Herrenschmidt,
Paul Mackerras, Michael Ellerman
Cc: Naveen N . Rao, linuxppc-dev, linux-kernel
When FL_SAVE_REGS is not set we get here via ftrace_caller()
which doesn't save all registers.
ftrace_caller() explicitely clears regs.msr, so we can rely
on it to know where we come from. We don't expect MSR register
to be 0 at all when involving ftrace.
Reported-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
Fixes: 40b035efe288 ("powerpc/ftrace: Implement CONFIG_DYNAMIC_FTRACE_WITH_ARGS")
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/include/asm/ftrace.h | 3 ++-
arch/powerpc/kernel/trace/ftrace_mprofile.S | 4 ++++
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/include/asm/ftrace.h b/arch/powerpc/include/asm/ftrace.h
index 70b457097098..ff034ae4e472 100644
--- a/arch/powerpc/include/asm/ftrace.h
+++ b/arch/powerpc/include/asm/ftrace.h
@@ -30,7 +30,8 @@ struct ftrace_regs {
static __always_inline struct pt_regs *arch_ftrace_get_regs(struct ftrace_regs *fregs)
{
- return &fregs->regs;
+ /* We clear regs.msr in ftrace_call */
+ return fregs->regs.msr ? &fregs->regs : NULL;
}
static __always_inline void ftrace_instruction_pointer_set(struct ftrace_regs *fregs,
diff --git a/arch/powerpc/kernel/trace/ftrace_mprofile.S b/arch/powerpc/kernel/trace/ftrace_mprofile.S
index 8443902d5a05..eb077270ec2f 100644
--- a/arch/powerpc/kernel/trace/ftrace_mprofile.S
+++ b/arch/powerpc/kernel/trace/ftrace_mprofile.S
@@ -205,6 +205,10 @@ _GLOBAL(ftrace_caller)
PPC_STL r0, _LINK(r1)
mr r4, r0
+ /* Clear MSR to flag as ftrace_caller versus frace_regs_caller */
+ li r8, 0
+ PPC_STL r8, _MSR(r1)
+
/* Load &pt_regs in r6 for call below */
addi r6, r1 ,STACK_FRAME_OVERHEAD
--
2.34.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v1 4/4] powerpc/ftrace: Style cleanup in ftrace_mprofile.S
2022-02-15 18:31 [PATCH v1 1/4] powerpc/ftrace: Also save r1 in ftrace_caller() Christophe Leroy
2022-02-15 18:31 ` [PATCH v1 2/4] powerpc/ftrace: Add recursion protection in prepare_ftrace_return() Christophe Leroy
2022-02-15 18:31 ` [PATCH v1 3/4] powerpc/ftrace: Have arch_ftrace_get_regs() return NULL unless FL_SAVE_REGS is set Christophe Leroy
@ 2022-02-15 18:31 ` Christophe Leroy
2022-02-16 10:22 ` Naveen N. Rao
2022-03-02 12:41 ` [PATCH v1 1/4] powerpc/ftrace: Also save r1 in ftrace_caller() Michael Ellerman
3 siblings, 1 reply; 6+ messages in thread
From: Christophe Leroy @ 2022-02-15 18:31 UTC (permalink / raw)
To: Steven Rostedt, Ingo Molnar, Benjamin Herrenschmidt,
Paul Mackerras, Michael Ellerman
Cc: Naveen N . Rao, linuxppc-dev, linux-kernel
Add some line breaks to better match the file's style, add
some space after comma and fix a couple of misplaced blanks.
Suggested-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/kernel/trace/ftrace_mprofile.S | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/kernel/trace/ftrace_mprofile.S b/arch/powerpc/kernel/trace/ftrace_mprofile.S
index eb077270ec2f..89639e64acd1 100644
--- a/arch/powerpc/kernel/trace/ftrace_mprofile.S
+++ b/arch/powerpc/kernel/trace/ftrace_mprofile.S
@@ -87,8 +87,9 @@ _GLOBAL(ftrace_regs_caller)
#endif
#ifdef CONFIG_LIVEPATCH_64
- mr r14,r7 /* remember old NIP */
+ mr r14, r7 /* remember old NIP */
#endif
+
/* Calculate ip from nip-4 into r3 for call below */
subi r3, r7, MCOUNT_INSN_SIZE
@@ -102,7 +103,7 @@ _GLOBAL(ftrace_regs_caller)
PPC_STL r11, _CCR(r1)
/* Load &pt_regs in r6 for call below */
- addi r6, r1 ,STACK_FRAME_OVERHEAD
+ addi r6, r1, STACK_FRAME_OVERHEAD
/* ftrace_call(r3, r4, r5, r6) */
.globl ftrace_regs_call
@@ -113,6 +114,7 @@ ftrace_regs_call:
/* Load ctr with the possibly modified NIP */
PPC_LL r3, _NIP(r1)
mtctr r3
+
#ifdef CONFIG_LIVEPATCH_64
cmpd r14, r3 /* has NIP been altered? */
#endif
@@ -196,7 +198,7 @@ _GLOBAL(ftrace_caller)
#ifdef CONFIG_LIVEPATCH_64
SAVE_GPR(14, r1)
- mr r14,r7 /* remember old NIP */
+ mr r14, r7 /* remember old NIP */
#endif
/* Calculate ip from nip-4 into r3 for call below */
subi r3, r7, MCOUNT_INSN_SIZE
@@ -210,7 +212,7 @@ _GLOBAL(ftrace_caller)
PPC_STL r8, _MSR(r1)
/* Load &pt_regs in r6 for call below */
- addi r6, r1 ,STACK_FRAME_OVERHEAD
+ addi r6, r1, STACK_FRAME_OVERHEAD
/* ftrace_call(r3, r4, r5, r6) */
.globl ftrace_call
@@ -220,6 +222,7 @@ ftrace_call:
PPC_LL r3, _NIP(r1)
mtctr r3
+
#ifdef CONFIG_LIVEPATCH_64
cmpd r14, r3 /* has NIP been altered? */
REST_GPR(14, r1)
@@ -244,6 +247,7 @@ ftrace_call:
/* Based on the cmpd above, if the NIP was altered handle livepatch */
bne- livepatch_handler
#endif
+
bctr /* jump after _mcount site */
#ifdef CONFIG_LIVEPATCH_64
--
2.34.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v1 4/4] powerpc/ftrace: Style cleanup in ftrace_mprofile.S
2022-02-15 18:31 ` [PATCH v1 4/4] powerpc/ftrace: Style cleanup in ftrace_mprofile.S Christophe Leroy
@ 2022-02-16 10:22 ` Naveen N. Rao
0 siblings, 0 replies; 6+ messages in thread
From: Naveen N. Rao @ 2022-02-16 10:22 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Christophe Leroy, Ingo Molnar,
Michael Ellerman, Paul Mackerras, Steven Rostedt
Cc: linuxppc-dev, linux-kernel
Christophe Leroy wrote:
> Add some line breaks to better match the file's style, add
> some space after comma and fix a couple of misplaced blanks.
>
> Suggested-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
> arch/powerpc/kernel/trace/ftrace_mprofile.S | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
Thanks. For the series:
Reviewed-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
- Naveen
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v1 1/4] powerpc/ftrace: Also save r1 in ftrace_caller()
2022-02-15 18:31 [PATCH v1 1/4] powerpc/ftrace: Also save r1 in ftrace_caller() Christophe Leroy
` (2 preceding siblings ...)
2022-02-15 18:31 ` [PATCH v1 4/4] powerpc/ftrace: Style cleanup in ftrace_mprofile.S Christophe Leroy
@ 2022-03-02 12:41 ` Michael Ellerman
3 siblings, 0 replies; 6+ messages in thread
From: Michael Ellerman @ 2022-03-02 12:41 UTC (permalink / raw)
To: Paul Mackerras, Christophe Leroy, Michael Ellerman,
Steven Rostedt, Ingo Molnar, Benjamin Herrenschmidt
Cc: Naveen N . Rao, linuxppc-dev, linux-kernel
On Tue, 15 Feb 2022 19:31:22 +0100, Christophe Leroy wrote:
> Also save r1 in ftrace_caller()
>
> r1 is needed during unwinding when the function_graph tracer
> is active.
>
>
Applied to powerpc/next.
[1/4] powerpc/ftrace: Also save r1 in ftrace_caller()
https://git.kernel.org/powerpc/c/34d8dac807f0ee3dc42ab45bdb284a3caf2b5ed1
[2/4] powerpc/ftrace: Add recursion protection in prepare_ftrace_return()
https://git.kernel.org/powerpc/c/df45a55788286c541449d82ee09fef3ac5ff77a1
[3/4] powerpc/ftrace: Have arch_ftrace_get_regs() return NULL unless FL_SAVE_REGS is set
https://git.kernel.org/powerpc/c/fc75f87337983229b7355d6b77f30fb6e7f359ee
[4/4] powerpc/ftrace: Style cleanup in ftrace_mprofile.S
https://git.kernel.org/powerpc/c/76b372814b088aeb76f0f753d968c8aa6d297f2a
cheers
^ permalink raw reply [flat|nested] 6+ messages in thread