[PATCH] parisc/ftrace: Fix function graph tracing disablement

[PATCH] parisc/ftrace: Fix function graph tracing disablement

[Josh Poimboeuf]

Due to an apparent copy-paste bug, the parisc implementation of
ftrace_disable_ftrace_graph_caller() doesn't actually do anything.  It
enables the (already-enabled) static key rather than disabling it.

The result is that after function graph tracing has been "disabled", any
subsequent (non-graph) function tracing will inadvertently also enable
the slow fgraph return address hijacking.

Fixes: 98f2926171ae ("parisc/ftrace: use static key to enable/disable function graph tracer")
Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
---
 arch/parisc/kernel/ftrace.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/parisc/kernel/ftrace.c b/arch/parisc/kernel/ftrace.c
index c91f9c2e61ed..f8d08eab7db8 100644
--- a/arch/parisc/kernel/ftrace.c
+++ b/arch/parisc/kernel/ftrace.c
@@ -87,7 +87,7 @@ int ftrace_enable_ftrace_graph_caller(void)
 
 int ftrace_disable_ftrace_graph_caller(void)
 {
-	static_key_enable(&ftrace_graph_enable.key);
+	static_key_disable(&ftrace_graph_enable.key);
 	return 0;
 }
 #endif
-- 
2.46.0

Re: [PATCH] parisc/ftrace: Fix function graph tracing disablement

[Steven Rostedt]

On Wed,  2 Oct 2024 14:32:04 -0700
Josh Poimboeuf <jpoimboe@kernel.org> wrote:

> Due to an apparent copy-paste bug, the parisc implementation of
> ftrace_disable_ftrace_graph_caller() doesn't actually do anything.  It
> enables the (already-enabled) static key rather than disabling it.
> 
> The result is that after function graph tracing has been "disabled", any
> subsequent (non-graph) function tracing will inadvertently also enable
> the slow fgraph return address hijacking.
> 
> Fixes: 98f2926171ae ("parisc/ftrace: use static key to enable/disable function graph tracer")
> Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
> ---
>  arch/parisc/kernel/ftrace.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/parisc/kernel/ftrace.c b/arch/parisc/kernel/ftrace.c
> index c91f9c2e61ed..f8d08eab7db8 100644
> --- a/arch/parisc/kernel/ftrace.c
> +++ b/arch/parisc/kernel/ftrace.c
> @@ -87,7 +87,7 @@ int ftrace_enable_ftrace_graph_caller(void)
>  
>  int ftrace_disable_ftrace_graph_caller(void)
>  {
> -	static_key_enable(&ftrace_graph_enable.key);
> +	static_key_disable(&ftrace_graph_enable.key);
>  	return 0;
>  }
>  #endif

Awfully quite :-/

I wonder if anyone cares about parisc today?

Hmm.

-- Steve

Re: [PATCH] parisc/ftrace: Fix function graph tracing disablement

[Josh Poimboeuf]

On Thu, Oct 10, 2024 at 03:48:40PM -0400, Steven Rostedt wrote:
> > +++ b/arch/parisc/kernel/ftrace.c
> > @@ -87,7 +87,7 @@ int ftrace_enable_ftrace_graph_caller(void)
> >  
> >  int ftrace_disable_ftrace_graph_caller(void)
> >  {
> > -	static_key_enable(&ftrace_graph_enable.key);
> > +	static_key_disable(&ftrace_graph_enable.key);
> >  	return 0;
> >  }
> >  #endif
> 
> Awfully quite :-/
> 
> I wonder if anyone cares about parisc today?
> 
> Hmm.

I still see some git activity in arch/parisc, so apparently so...

I only stumbled on this by accident (code inspection) while making
another unrelated change.

-- 
Josh

Re: [PATCH] parisc/ftrace: Fix function graph tracing disablement

[Helge Deller]

On 10/10/24 21:56, Josh Poimboeuf wrote:
> On Thu, Oct 10, 2024 at 03:48:40PM -0400, Steven Rostedt wrote:
>>> +++ b/arch/parisc/kernel/ftrace.c
>>> @@ -87,7 +87,7 @@ int ftrace_enable_ftrace_graph_caller(void)
>>>
>>>   int ftrace_disable_ftrace_graph_caller(void)
>>>   {
>>> -	static_key_enable(&ftrace_graph_enable.key);
>>> +	static_key_disable(&ftrace_graph_enable.key);
>>>   	return 0;
>>>   }
>>>   #endif
>>
>> Awfully quite :-/
>>
>> I wonder if anyone cares about parisc today?

Actually I do, and yes we have users.
Of course in reality that's "fun" users and not "productive" usage.

 From programming perspective parisc is quite interesting since it
has an upwards-growing stack, limited ways of atomic ops,
and some awkward cache dependencies, so I regularly find
issues in generic Linux code which would not be found otherwise.

> I still see some git activity in arch/parisc, so apparently so...
>
> I only stumbled on this by accident (code inspection) while making
> another unrelated change.

Thanks for your patch!
I'll apply it in my for-next git tree and push upstream at some point
(unless someone else takes it before me).

Helge

Re: [PATCH] parisc/ftrace: Fix function graph tracing disablement

[Steven Rostedt]

On Thu, 10 Oct 2024 22:25:13 +0200
Helge Deller <deller@gmx.de> wrote:
> 
> Actually I do, and yes we have users.
> Of course in reality that's "fun" users and not "productive" usage.

We welcome those "fun" users as well!

> 
>  From programming perspective parisc is quite interesting since it
> has an upwards-growing stack, limited ways of atomic ops,
> and some awkward cache dependencies, so I regularly find
> issues in generic Linux code which would not be found otherwise.
> 
> > I still see some git activity in arch/parisc, so apparently so...
> >
> > I only stumbled on this by accident (code inspection) while making
> > another unrelated change.  
> 
> Thanks for your patch!
> I'll apply it in my for-next git tree and push upstream at some point
> (unless someone else takes it before me).

For architecture patches like this, even if it is obvious, I rather it go
through them, as I wouldn't even compile test it.

Please take it through your tree.

Thanks,

-- Steve