Linux Trace Kernel
 help / color / mirror / Atom feed
* [PATCH] perf/ftrace: Fix WARNING in __unregister_ftrace_function
@ 2026-05-13 16:16 Rik van Riel
  2026-05-13 16:33 ` Steven Rostedt
  0 siblings, 1 reply; 5+ messages in thread
From: Rik van Riel @ 2026-05-13 16:16 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Masami Hiramatsu, Mathieu Desnoyers, linux-kernel,
	linux-trace-kernel, kernel-team

perf_ftrace_function_unregister() unconditionally calls
unregister_ftrace_function() without checking whether the ftrace_ops
was ever successfully registered. This triggers a WARN_ON in
__unregister_ftrace_function() when the ops doesn't have
FTRACE_OPS_FL_ENABLED set.

This can happen during perf_event_alloc() error cleanup when
perf_trace_destroy() is called via __free_event() on an event whose
ftrace_ops registration failed or was already torn down by
perf_try_init_event()'s err_destroy path.

The call path is:
  perf_event_alloc() error cleanup
    -> __free_event()
      -> event->destroy() [tp_perf_event_destroy]
        -> perf_trace_destroy()
          -> perf_trace_event_close()
            -> TRACE_REG_PERF_CLOSE
              -> perf_ftrace_function_unregister()
                -> unregister_ftrace_function()
                  -> __unregister_ftrace_function()
                    -> WARN_ON(!(ops->flags & FTRACE_OPS_FL_ENABLED))

Fix this by checking FTRACE_OPS_FL_ENABLED before attempting to
unregister. If the ops is not enabled, just free the filter and
return success.

Assisted-by: Claude:claude-opus-4.7 syzkaller
Signed-off-by: Rik van Riel <riel@surriel.com>
---
 kernel/trace/trace_event_perf.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/kernel/trace/trace_event_perf.c b/kernel/trace/trace_event_perf.c
index a6bb7577e8c5..b9e33ae24867 100644
--- a/kernel/trace/trace_event_perf.c
+++ b/kernel/trace/trace_event_perf.c
@@ -497,7 +497,14 @@ static int perf_ftrace_function_register(struct perf_event *event)
 static int perf_ftrace_function_unregister(struct perf_event *event)
 {
 	struct ftrace_ops *ops = &event->ftrace_ops;
-	int ret = unregister_ftrace_function(ops);
+	int ret;
+
+	if (!(ops->flags & FTRACE_OPS_FL_ENABLED)) {
+		ftrace_free_filter(ops);
+		return 0;
+	}
+
+	ret = unregister_ftrace_function(ops);
 	ftrace_free_filter(ops);
 	return ret;
 }
-- 
2.53.0-Meta


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] perf/ftrace: Fix WARNING in __unregister_ftrace_function
  2026-05-13 16:16 [PATCH] perf/ftrace: Fix WARNING in __unregister_ftrace_function Rik van Riel
@ 2026-05-13 16:33 ` Steven Rostedt
  2026-05-13 17:24   ` [PATCH v2] " Rik van Riel
  0 siblings, 1 reply; 5+ messages in thread
From: Steven Rostedt @ 2026-05-13 16:33 UTC (permalink / raw)
  To: Rik van Riel
  Cc: Masami Hiramatsu, Mathieu Desnoyers, linux-kernel,
	linux-trace-kernel, kernel-team

On Wed, 13 May 2026 12:16:07 -0400
Rik van Riel <riel@surriel.com> wrote:

> diff --git a/kernel/trace/trace_event_perf.c b/kernel/trace/trace_event_perf.c
> index a6bb7577e8c5..b9e33ae24867 100644
> --- a/kernel/trace/trace_event_perf.c
> +++ b/kernel/trace/trace_event_perf.c
> @@ -497,7 +497,14 @@ static int perf_ftrace_function_register(struct perf_event *event)
>  static int perf_ftrace_function_unregister(struct perf_event *event)
>  {
>  	struct ftrace_ops *ops = &event->ftrace_ops;
> -	int ret = unregister_ftrace_function(ops);
> +	int ret;
> +
> +	if (!(ops->flags & FTRACE_OPS_FL_ENABLED)) {
> +		ftrace_free_filter(ops);
> +		return 0;
> +	}
> +
> +	ret = unregister_ftrace_function(ops);
>  	ftrace_free_filter(ops);
>  	return ret;
>  }
> -- 

Instead of duplicating code, what about doing:

static int perf_ftrace_function_unregister(struct perf_event *event)
{
	struct ftrace_ops *ops = &event->ftrace_ops;
	int ret = 0;

	if (ops->flags & FTRACE_OPS_FL_ENABLED)
		ret = unregister_ftrace_function(ops);

	ftrace_free_filter(ops);
	return ret;
}


?

-- Steve

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] perf/ftrace: Fix WARNING in __unregister_ftrace_function
  2026-05-13 16:33 ` Steven Rostedt
@ 2026-05-13 17:24   ` Rik van Riel
  2026-05-13 18:11     ` Steven Rostedt
  0 siblings, 1 reply; 5+ messages in thread
From: Rik van Riel @ 2026-05-13 17:24 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Masami Hiramatsu, Mathieu Desnoyers, linux-kernel,
	linux-trace-kernel, kernel-team

On Wed, 13 May 2026 12:33:44 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> 
> Instead of duplicating code, what about doing:

That is much nicer. Thank you!

---8<---

From 9de86227b917c49315b7b67aac3a83afae8d792d Mon Sep 17 00:00:00 2001
From: Rik van Riel <riel@meta.com>
Date: Sat, 25 Apr 2026 03:33:54 -0700
Subject: [PATCH] perf/ftrace: Fix WARNING in __unregister_ftrace_function

perf_ftrace_function_unregister() unconditionally calls
unregister_ftrace_function() without checking whether the ftrace_ops
was ever successfully registered. This triggers a WARN_ON in
__unregister_ftrace_function() when the ops doesn't have
FTRACE_OPS_FL_ENABLED set.

This can happen during perf_event_alloc() error cleanup when
perf_trace_destroy() is called via __free_event() on an event whose
ftrace_ops registration failed or was already torn down by
perf_try_init_event()'s err_destroy path.

The call path is:
  perf_event_alloc() error cleanup
    -> __free_event()
      -> event->destroy() [tp_perf_event_destroy]
        -> perf_trace_destroy()
          -> perf_trace_event_close()
            -> TRACE_REG_PERF_CLOSE
              -> perf_ftrace_function_unregister()
                -> unregister_ftrace_function()
                  -> __unregister_ftrace_function()
                    -> WARN_ON(!(ops->flags & FTRACE_OPS_FL_ENABLED))

Fix this by checking FTRACE_OPS_FL_ENABLED before attempting to
unregister. If the ops is not enabled, just free the filter and
return success.

Assisted-by: Claude:claude-opus-4.7 syzkaller
Signed-off-by: Rik van Riel <riel@surriel.com>
---
 kernel/trace/trace_event_perf.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/kernel/trace/trace_event_perf.c b/kernel/trace/trace_event_perf.c
index a6bb7577e8c5..58e1b427b576 100644
--- a/kernel/trace/trace_event_perf.c
+++ b/kernel/trace/trace_event_perf.c
@@ -497,7 +497,11 @@ static int perf_ftrace_function_register(struct perf_event *event)
 static int perf_ftrace_function_unregister(struct perf_event *event)
 {
 	struct ftrace_ops *ops = &event->ftrace_ops;
-	int ret = unregister_ftrace_function(ops);
+	int ret = 0;
+
+	if (ops->flags & FTRACE_OPS_FL_ENABLED)
+		ret = unregister_ftrace_function(ops);
+
 	ftrace_free_filter(ops);
 	return ret;
 }
-- 
2.52.0



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] perf/ftrace: Fix WARNING in __unregister_ftrace_function
  2026-05-13 17:24   ` [PATCH v2] " Rik van Riel
@ 2026-05-13 18:11     ` Steven Rostedt
  0 siblings, 0 replies; 5+ messages in thread
From: Steven Rostedt @ 2026-05-13 18:11 UTC (permalink / raw)
  To: Rik van Riel
  Cc: Masami Hiramatsu, Mathieu Desnoyers, linux-kernel,
	linux-trace-kernel, kernel-team

On Wed, 13 May 2026 13:24:45 -0400
Rik van Riel <riel@surriel.com> wrote:

> From 9de86227b917c49315b7b67aac3a83afae8d792d Mon Sep 17 00:00:00 2001
> From: Rik van Riel <riel@meta.com>
> Date: Sat, 25 Apr 2026 03:33:54 -0700
> Subject: [PATCH] perf/ftrace: Fix WARNING in __unregister_ftrace_function
> 

Can you resend this as a normal patch so that it can be picked up by patchwork.

Otherwise it will be ignored.

Thanks,

-- Steve

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v2] perf/ftrace: Fix WARNING in __unregister_ftrace_function
@ 2026-05-13 20:19 Rik van Riel
  0 siblings, 0 replies; 5+ messages in thread
From: Rik van Riel @ 2026-05-13 20:19 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Masami Hiramatsu, Mathieu Desnoyers, linux-kernel,
	linux-trace-kernel, kernel-team

perf_ftrace_function_unregister() unconditionally calls
unregister_ftrace_function() without checking whether the ftrace_ops
was ever successfully registered. This triggers a WARN_ON in
__unregister_ftrace_function() when the ops doesn't have
FTRACE_OPS_FL_ENABLED set.

This can happen during perf_event_alloc() error cleanup when
perf_trace_destroy() is called via __free_event() on an event whose
ftrace_ops registration failed or was already torn down by
perf_try_init_event()'s err_destroy path.

The call path is:
  perf_event_alloc() error cleanup
    -> __free_event()
      -> event->destroy() [tp_perf_event_destroy]
        -> perf_trace_destroy()
          -> perf_trace_event_close()
            -> TRACE_REG_PERF_CLOSE
              -> perf_ftrace_function_unregister()
                -> unregister_ftrace_function()
                  -> __unregister_ftrace_function()
                    -> WARN_ON(!(ops->flags & FTRACE_OPS_FL_ENABLED))

Fix this by checking FTRACE_OPS_FL_ENABLED before attempting to
unregister. If the ops is not enabled, just free the filter and
return success.

Assisted-by: Claude:claude-opus-4.7 syzkaller
Signed-off-by: Rik van Riel <riel@surriel.com>
---
 kernel/trace/trace_event_perf.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/kernel/trace/trace_event_perf.c b/kernel/trace/trace_event_perf.c
index a6bb7577e8c5..58e1b427b576 100644
--- a/kernel/trace/trace_event_perf.c
+++ b/kernel/trace/trace_event_perf.c
@@ -497,7 +497,11 @@ static int perf_ftrace_function_register(struct perf_event *event)
 static int perf_ftrace_function_unregister(struct perf_event *event)
 {
 	struct ftrace_ops *ops = &event->ftrace_ops;
-	int ret = unregister_ftrace_function(ops);
+	int ret = 0;
+
+	if (ops->flags & FTRACE_OPS_FL_ENABLED)
+		ret = unregister_ftrace_function(ops);
+
 	ftrace_free_filter(ops);
 	return ret;
 }
-- 
2.52.0



^ permalink raw reply related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-05-13 20:19 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-13 16:16 [PATCH] perf/ftrace: Fix WARNING in __unregister_ftrace_function Rik van Riel
2026-05-13 16:33 ` Steven Rostedt
2026-05-13 17:24   ` [PATCH v2] " Rik van Riel
2026-05-13 18:11     ` Steven Rostedt
  -- strict thread matches above, loose matches on Subject: below --
2026-05-13 20:19 Rik van Riel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox