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

[sashiko-bot@kernel.org]

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