On Wed, 2 Sep 2026 09:55:01 -0400 Steven Rostedt <[email protected]> wrote:
> 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. > Looks good to me. Reviewed-by: Masami Hiramatsu (Google) <[email protected]> Thanks, > 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]> > --- > Changes since v1: https://patch.msgid.link/20260901202020.09a1119a@robin > > - Added smp_load_acquire() and smp_store_release() to make sure that > the flags value that is compared is really updated after the code has > been initialized. (Sashiko) > > kernel/trace/ftrace.c | 13 +++++++++++-- > 1 file changed, 11 insertions(+), 2 deletions(-) > > diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c > index c7cf36f2dd7b..53d5db60bfa5 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 > @@ -159,11 +161,18 @@ const struct ftrace_ops ftrace_nop_ops = { > static inline void ftrace_ops_init(struct ftrace_ops *ops) > { > #ifdef CONFIG_DYNAMIC_FTRACE > - if (!(ops->flags & FTRACE_OPS_FL_INITIALIZED)) { > + unsigned long flags = smp_load_acquire(&ops->flags); > + > + if (!(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; > - ops->flags |= FTRACE_OPS_FL_INITIALIZED; > + flags = ops->flags | FTRACE_OPS_FL_INITIALIZED; > + smp_store_release(&ops->flags, flags); > } > #endif > } > -- > 2.53.0 > -- Masami Hiramatsu (Google) <[email protected]>
