[PATCH] ftrace: Synchronize the initialization of ftrace_ops

[PATCH] ftrace: Synchronize the initialization of ftrace_ops

[Steven Rostedt]

From: Steven Rostedt <rostedt@goodmis.org>

There's some internal state that ftrace_ops needs to have set, but since
it can be declared outside of the ftrace.c code, it calls
ftrace_ops_init() on the ops in every global function. The issue is that
if two tasks call it on the same ops at the same time it is possible to
have the initialization of one corrupt the initialization of the other
call.

Create a ops_mutex to use to synchronize every initialization of the
ftrace_ops. The mutex is taken within checking the ftrace_ops flag that
states it was initializied but the flag is checked again after the mutex
has been taken. Checking first outside the mutex allows it to shortcut
having to take the mutex. But then the check needs to be done again after
the mute is taken in case of races.

Cc: stable@vger.kernel.org
Fixes: f04f24fb7e48d ("ftrace, kprobes: Fix a deadlock on ftrace_regex_lock")
Reported-by: sashiko-bot@kernel.org
Close: https://lore.kernel.org/all/20260829025528.49A831F000E9@smtp.kernel.org/
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 kernel/trace/ftrace.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index c7cf36f2dd7b..0a49a1819c4d 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -75,6 +75,8 @@
 	.func_hash		= &opsname.local_hash,			\
 	.local_hash.regex_lock	= __MUTEX_INITIALIZER(opsname.local_hash.regex_lock), \
 	.subop_list		= LIST_HEAD_INIT(opsname.subop_list),
+/* Used only to synchronize the initialization of ftrace_ops */
+static DEFINE_MUTEX(ops_mutex);
 #else
 #define INIT_OPS_HASH(opsname)
 #endif
@@ -160,6 +162,10 @@ static inline void ftrace_ops_init(struct ftrace_ops *ops)
 {
 #ifdef CONFIG_DYNAMIC_FTRACE
 	if (!(ops->flags & FTRACE_OPS_FL_INITIALIZED)) {
+		guard(mutex)(&ops_mutex);
+		/* Could have been initialized before lock taken */
+		if (unlikely(ops->flags & FTRACE_OPS_FL_INITIALIZED))
+			return;
 		mutex_init(&ops->local_hash.regex_lock);
 		INIT_LIST_HEAD(&ops->subop_list);
 		ops->func_hash = &ops->local_hash;
-- 
2.53.0

Re: [PATCH] ftrace: Synchronize the initialization of ftrace_ops

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The newly introduced `ops_mutex` logic implements a Double-Checked Locking pattern for `ftrace_ops` initialization but misses required memory barriers, allowing concurrent threads to observe a partially initialized structure on weakly-ordered architectures.
--

commit 0d99940712b506309368df1237eae674da6b9bc1
Author: Steven Rostedt <rostedt@goodmis.org>

ftrace: Synchronize the initialization of ftrace_ops

This commit introduces a mutex to synchronize the initialization of ftrace_ops
across multiple tasks. It implements a fast-path optimization by checking the
FTRACE_OPS_FL_INITIALIZED flag before acquiring the lock.

Link: https://lore.kernel.org/all/20260829025528.49A831F000E9@smtp.kernel.org/

> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index f9d80c7bd9f16..0dcc1ec1a366f 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
[ ... ]
> @@ -160,6 +162,10 @@ static inline void ftrace_ops_init(struct ftrace_ops *ops)
>  {
>  #ifdef CONFIG_DYNAMIC_FTRACE
>  	if (!(ops->flags & FTRACE_OPS_FL_INITIALIZED)) {
> +		guard(mutex)(&ops_mutex);
> +		/* Could have been initialized before lock taken */
> +		if (unlikely(ops->flags & FTRACE_OPS_FL_INITIALIZED))
> +			return;
>  		mutex_init(&ops->local_hash.regex_lock);
>  		INIT_LIST_HEAD(&ops->subop_list);
>  		ops->func_hash = &ops->local_hash;

[Severity: High]
Does this double-checked locking pattern introduce a memory ordering
regression on weakly-ordered architectures?

The initial lock-free fast-path check !(ops->flags & FTRACE_OPS_FL_INITIALIZED)
uses a plain load, while the subsequent completion signal
ops->flags |= FTRACE_OPS_FL_INITIALIZED (which occurs just after the quoted
lines) uses a plain store.

Without an acquire memory barrier on the load side and a release memory
barrier on the store side, is it possible for memory accesses to be
reordered by the CPU?

If Thread A completes the initialization and writes the flag, could
Thread B bypass the mutex by observing the initialized flag as true, but
still read a stale uninitialized or NULL value for ops->func_hash?

If Thread B then immediately dereferences ops->func_hash, for instance
in ftrace_startup_subops():

kernel/trace/ftrace.c:ftrace_startup_subops() {
    ...
	if (!ops->func_hash->filter_hash)
    ...
}

Would this result in an invalid memory access and a kernel panic?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260901202020.09a1119a@robin?part=1

Re: [PATCH] ftrace: Synchronize the initialization of ftrace_ops

[Steven Rostedt]

On Wed, 02 Sep 2026 00:33:43 +0000
sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] The newly introduced `ops_mutex` logic implements a Double-Checked Locking pattern for `ftrace_ops` initialization but misses required memory barriers, allowing concurrent threads to observe a partially initialized structure on weakly-ordered architectures.
> --
> 
> commit 0d99940712b506309368df1237eae674da6b9bc1
> Author: Steven Rostedt <rostedt@goodmis.org>
> 
> ftrace: Synchronize the initialization of ftrace_ops
> 
> This commit introduces a mutex to synchronize the initialization of ftrace_ops
> across multiple tasks. It implements a fast-path optimization by checking the
> FTRACE_OPS_FL_INITIALIZED flag before acquiring the lock.
> 
> Link: https://lore.kernel.org/all/20260829025528.49A831F000E9@smtp.kernel.org/
> 
> > diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> > index f9d80c7bd9f16..0dcc1ec1a366f 100644
> > --- a/kernel/trace/ftrace.c
> > +++ b/kernel/trace/ftrace.c  
> [ ... ]
> > @@ -160,6 +162,10 @@ static inline void ftrace_ops_init(struct ftrace_ops *ops)
> >  {
> >  #ifdef CONFIG_DYNAMIC_FTRACE
> >  	if (!(ops->flags & FTRACE_OPS_FL_INITIALIZED)) {
> > +		guard(mutex)(&ops_mutex);
> > +		/* Could have been initialized before lock taken */
> > +		if (unlikely(ops->flags & FTRACE_OPS_FL_INITIALIZED))
> > +			return;
> >  		mutex_init(&ops->local_hash.regex_lock);
> >  		INIT_LIST_HEAD(&ops->subop_list);
> >  		ops->func_hash = &ops->local_hash;  
> 
> [Severity: High]
> Does this double-checked locking pattern introduce a memory ordering
> regression on weakly-ordered architectures?
> 
> The initial lock-free fast-path check !(ops->flags & FTRACE_OPS_FL_INITIALIZED)
> uses a plain load, while the subsequent completion signal
> ops->flags |= FTRACE_OPS_FL_INITIALIZED (which occurs just after the quoted
> lines) uses a plain store.
> 
> Without an acquire memory barrier on the load side and a release memory
> barrier on the store side, is it possible for memory accesses to be
> reordered by the CPU?

Yeah yeah, I figured this would be triggered. I'll update with
smp_load_acquire() and smp_store_release().

-- Steve