kernel/trace/trace_event_perf.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
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.0On Wed, 13 May 2026 16:19:16 -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..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;
> +
Because this is different than unregister_ftrace_function() where it will
not fail if the ops is not registered, it deserves a comment. Something
like:
/*
* Perf will call this unconditionally even if the ops is not
* enabled. The unregister_ftrace_function() will warn if called
* when not enabled. Just bypass the unregistering if ops isn't
* enabled here.
*/
Thanks,
-- Steve
> + if (ops->flags & FTRACE_OPS_FL_ENABLED)
> + ret = unregister_ftrace_function(ops);
> +
> ftrace_free_filter(ops);
> return ret;
> }
Rik,
Can you reply to Sashiko here?
The below comes from Sashiko: https://sashiko.dev/?list=org.kernel.vger.linux-trace-kernel#/patchset/20260513161916.04151502%40fangorn
Thanks,
-- Steve
> commit f90ac36f39b79a19c015522bb1caf7ef903caf02
> Author: Rik van Riel <riel@surriel.com>
> perf/ftrace: Fix WARNING in __unregister_ftrace_function
> This commit attempts to fix a WARN_ON triggered during perf_event_alloc() error
> cleanup by checking FTRACE_OPS_FL_ENABLED before unregistering the ftrace
> function. The warning occurs because the ftrace_ops registration failed or was
> torn down by perf_try_init_event() earlier, but the event->destroy callback is
> executed again during the error path.
> > diff --git a/kernel/trace/trace_event_perf.c b/kernel/trace/trace_event_perf.c
> > index 61e3a2620fa3c..8d64f17135e8f 100644
> > --- a/kernel/trace/trace_event_perf.c
> > +++ b/kernel/trace/trace_event_perf.c
> > @@ -495,7 +495,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);
> Does calling ftrace_free_filter() unconditionally here mask a double-teardown
> regression while leaving the underlying double-free active?
> In perf_try_init_event(), if a PMU event_init() succeeds but a subsequent
> capability check fails, it explicitly calls event->destroy(event) to roll back:
> kernel/events/core.c:perf_try_init_event() {
> ...
> if (ret && event->destroy)
> event->destroy(event);
> ...
> }
> However, it does not set event->destroy to NULL.
> When the event is later freed in error paths, the stale event->destroy callback
> executes a second time.
> If ftrace_free_filter() is called twice, won't it result in a direct
> double-free of ops->func_hash->filter_hash, since ftrace_free_filter() frees
> the hash but does not nullify the pointers?
> Additionally, for regular tracepoint events, wouldn't the second teardown
> execute perf_trace_event_unreg() again?
> This could double-decrement total_ref_count, potentially freeing the shared
> perf_trace_buf while other events are active, and trigger an unbalanced
> module_put() leading to module refcount underflows.
> Should the root cause be addressed by setting event->destroy = NULL in
> perf_try_init_event() immediately after invoking it?
> > return ret;
> > }
On Wed, 2026-05-20 at 16:41 -0400, Steven Rostedt wrote:
>
> Rik,
>
> Can you reply to Sashiko here?
>
> The below comes from Sashiko:
> https://sashiko.dev/?list=org.kernel.vger.linux-trace-kernel#/patchset/20260513161916.04151502%40fangorn
>
> Thanks,
>
> -- Steve
>
>
> > commit f90ac36f39b79a19c015522bb1caf7ef903caf02
> > Author: Rik van Riel <riel@surriel.com>
> > perf/ftrace: Fix WARNING in __unregister_ftrace_function
> > This commit attempts to fix a WARN_ON triggered during
> > perf_event_alloc() error
> > cleanup by checking FTRACE_OPS_FL_ENABLED before unregistering the
> > ftrace
> > function. The warning occurs because the ftrace_ops registration
> > failed or was
> > torn down by perf_try_init_event() earlier, but the event->destroy
> > callback is
> > executed again during the error path.
> > > diff --git a/kernel/trace/trace_event_perf.c
> > > b/kernel/trace/trace_event_perf.c
> > > index 61e3a2620fa3c..8d64f17135e8f 100644
> > > --- a/kernel/trace/trace_event_perf.c
> > > +++ b/kernel/trace/trace_event_perf.c
> > > @@ -495,7 +495,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);
>
> > Does calling ftrace_free_filter() unconditionally here mask a
> > double-teardown
> > regression while leaving the underlying double-free active?
I don't see how calling ftrace_free_filter() twice would
call issues, given that it sets the ->*_hash values to
EMPTY_HASH:
void ftrace_free_filter(struct ftrace_ops *ops)
{
ftrace_ops_init(ops);
if (WARN_ON(ops->flags & FTRACE_OPS_FL_ENABLED))
return;
free_ftrace_hash(ops->func_hash->filter_hash);
free_ftrace_hash(ops->func_hash->notrace_hash);
ops->func_hash->filter_hash = EMPTY_HASH;
ops->func_hash->notrace_hash = EMPTY_HASH;
}
void free_ftrace_hash(struct ftrace_hash *hash)
{
if (!hash || hash == EMPTY_HASH)
return;
..
> > In perf_try_init_event(), if a PMU event_init() succeeds but a
> > subsequent
> > capability check fails, it explicitly calls event->destroy(event)
> > to roll back:
> > kernel/events/core.c:perf_try_init_event() {
> > ...
> > if (ret && event->destroy)
> > event->destroy(event);
> > ...
> > }
The error handling there all seems to "goto err_destroy"
err_destroy:
if (event->destroy) {
event->destroy(event);
event->destroy = NULL;
}
> > However, it does not set event->destroy to NULL.
... but it does?
I am not sure what code Sashiko is looking at,
but it does not look like the code I just pulled.
Is there a different tree I should be looking at
than upstream Linus?
--
All Rights Reversed.
On Fri, 22 May 2026 16:39:41 -0400
Rik van Riel <riel@surriel.com> wrote:
> On Wed, 2026-05-20 at 16:41 -0400, Steven Rostedt wrote:
> >
> > Rik,
> >
> > Can you reply to Sashiko here?
> >
> > The below comes from Sashiko:
> > https://sashiko.dev/?list=org.kernel.vger.linux-trace-kernel#/patchset/20260513161916.04151502%40fangorn
> >
> > Thanks,
> >
> > -- Steve
> >
> >
> > > commit f90ac36f39b79a19c015522bb1caf7ef903caf02
> > > Author: Rik van Riel <riel@surriel.com>
> > > perf/ftrace: Fix WARNING in __unregister_ftrace_function
> > > This commit attempts to fix a WARN_ON triggered during
> > > perf_event_alloc() error
> > > cleanup by checking FTRACE_OPS_FL_ENABLED before unregistering the
> > > ftrace
> > > function. The warning occurs because the ftrace_ops registration
> > > failed or was
> > > torn down by perf_try_init_event() earlier, but the event->destroy
> > > callback is
> > > executed again during the error path.
> > > > diff --git a/kernel/trace/trace_event_perf.c
> > > > b/kernel/trace/trace_event_perf.c
> > > > index 61e3a2620fa3c..8d64f17135e8f 100644
> > > > --- a/kernel/trace/trace_event_perf.c
> > > > +++ b/kernel/trace/trace_event_perf.c
> > > > @@ -495,7 +495,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);
> >
> > > Does calling ftrace_free_filter() unconditionally here mask a
> > > double-teardown
> > > regression while leaving the underlying double-free active?
>
> I don't see how calling ftrace_free_filter() twice would
> call issues, given that it sets the ->*_hash values to
> EMPTY_HASH:
>
> void ftrace_free_filter(struct ftrace_ops *ops)
> {
> ftrace_ops_init(ops);
> if (WARN_ON(ops->flags & FTRACE_OPS_FL_ENABLED))
> return;
> free_ftrace_hash(ops->func_hash->filter_hash);
> free_ftrace_hash(ops->func_hash->notrace_hash);
> ops->func_hash->filter_hash = EMPTY_HASH;
> ops->func_hash->notrace_hash = EMPTY_HASH;
> }
>
> void free_ftrace_hash(struct ftrace_hash *hash)
> {
> if (!hash || hash == EMPTY_HASH)
> return;
> ..
>
Yeah, confirmed.
>
> > > In perf_try_init_event(), if a PMU event_init() succeeds but a
> > > subsequent
> > > capability check fails, it explicitly calls event->destroy(event)
> > > to roll back:
> > > kernel/events/core.c:perf_try_init_event() {
> > > ...
> > > if (ret && event->destroy)
> > > event->destroy(event);
> > > ...
> > > }
>
> The error handling there all seems to "goto err_destroy"
>
> err_destroy:
> if (event->destroy) {
> event->destroy(event);
> event->destroy = NULL;
> }
>
>
> > > However, it does not set event->destroy to NULL.
>
> ... but it does?
>
> I am not sure what code Sashiko is looking at,
> but it does not look like the code I just pulled.
Indeed.
>
> Is there a different tree I should be looking at
> than upstream Linus?
You can see the baseline info if you expand the collapsed triangle.
Anyway, it said:
linux-trace/HEAD (70575e77839f4c5337ce2653b39b86bb365a870e)
So that is linux-trace/master.
commit 70575e77839f4c5337ce2653b39b86bb365a870e (linux-trace/master)
Merge: 7bc6e90d7aa4 a43ae8057cc1
Author: Linus Torvalds <torvalds@linux-foundation.org>
Date: Fri Sep 30 09:41:34 2022 -0700
Merge tag 'for_linus' of git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost
Hmm, this is too old. And linux-trace/master is not used anymore.
Reported to Sashiko.
https://github.com/sashiko-dev/sashiko/issues/218
Thank you,
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
On Mon, 25 May 2026 14:39:47 +0900
Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:
> > The error handling there all seems to "goto err_destroy"
> >
> > err_destroy:
> > if (event->destroy) {
> > event->destroy(event);
> > event->destroy = NULL;
> > }
> >
> >
> > > > However, it does not set event->destroy to NULL.
> >
> > ... but it does?
> >
> > I am not sure what code Sashiko is looking at,
> > but it does not look like the code I just pulled.
>
> Indeed.
>
> >
> > Is there a different tree I should be looking at
> > than upstream Linus?
>
> You can see the baseline info if you expand the collapsed triangle.
> Anyway, it said:
>
> linux-trace/HEAD (70575e77839f4c5337ce2653b39b86bb365a870e)
>
> So that is linux-trace/master.
>
> commit 70575e77839f4c5337ce2653b39b86bb365a870e (linux-trace/master)
> Merge: 7bc6e90d7aa4 a43ae8057cc1
> Author: Linus Torvalds <torvalds@linux-foundation.org>
> Date: Fri Sep 30 09:41:34 2022 -0700
>
> Merge tag 'for_linus' of git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost
>
>
> Hmm, this is too old. And linux-trace/master is not used anymore.
Hmm, I probably should delete that branch.
Thanks,
-- Steve
On Wed, 13 May 2026 16:19:16 -0400
Rik van Riel <riel@surriel.com> wrote:
> 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>
Looks good to me.
Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Fixes: ced39002f5ea ("ftrace, perf: Add support to use function tracepoint in perf")
Thanks,
> ---
> 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
>
>
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
© 2016 - 2026 Red Hat, Inc.