ftrace: Synchronize the initialization of ftrace_ops

PATCH Superseded Steven Rostedt 2 participants 3 messages Sep 2

Subsystems (2)

Tracing ftrace

lore

Share & tools

lore

Steven Rostedt

author maintainer

2026-09-02 00:20 UTC #0 lore raw

PATCH 1/1 ftrace: Synchronize the initialization of ftrace_ops

From: Steven Rostedt <[email protected]>

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 [email protected]

Fixes f04f24fb7e48d ("ftrace, kprobes: Fix a deadlock on ftrace_regex_lock")

Reported-by [email protected]

Close: https://lore.kernel.org/all/[email protected]/

Signed-off-by Steven Rostedt <[email protected]>

---
 kernel/trace/ftrace.c | 6 ++++++
 1 file changed, 6 insertions(+)

kernel/trace/ftrace.c +6 -0

index c7cf36f2dd7b..0a49a1819c4d 100644
@@ -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;

Signature · 1 line

2.53.0

[email protected]

AI review

2026-09-02 00:33 UTC #1 lore raw

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…

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 <[email protected]>

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/[email protected]/ open here

Quoted 4 lines

kernel/trace/ftrace.c +0 -0
index f9d80c7bd9f16..0dcc1ec1a366f 100644

[ ... ]

kernel/trace/ftrace.c static inline void ftrace_ops_init(struct ftrace_ops *ops) · line 171

Quoted 11 lines

kernel/trace/ftrace.c +4 -0

@@ -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?

Signature · 1 line

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

Steven Rostedt

author maintainer

2026-09-02 00:45 UTC #2 lore raw

Quoted 36 lines · On Wed, 02 Sep 2026 00:33:43 +0000 [email protected] 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 <[email protected]>
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/[email protected]/ open here
Quoted 4 lines
kernel/trace/ftrace.c +0 -0
index f9d80c7bd9f16..0dcc1ec1a366f 100644
[ ... ]
kernel/trace/ftrace.c static inline void ftrace_ops_init(struct ftrace_ops *ops) · line 171
Quoted 11 lines
kernel/trace/ftrace.c +4 -0
@@ -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?

Quoted 8 lines

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