kevin/linux
1
Fork 0
Code

tracing: fgraph: Fix to add new fgraph_ops to array after ftrace_startup_subops()

Browse Source 
Since the register_ftrace_graph() assigns a new fgraph_ops to
fgraph_array before registring it by ftrace_startup_subops(), the new
fgraph_ops can be used in function_graph_enter().

In most cases, it is still OK because those fgraph_ops's hashtable is
already initialized by ftrace_set_filter*() etc.

But if a user registers a new fgraph_ops which does not initialize the
hash list, ftrace_ops_test() in function_graph_enter() causes a NULL
pointer dereference BUG because fgraph_ops->ops.func_hash is NULL.

This can be reproduced by the below commands because function profiler's
fgraph_ops does not initialize the hash list;

 # cd /sys/kernel/tracing
 # echo function_graph > current_tracer
 # echo 1 > function_profile_enabled

To fix this problem, add a new fgraph_ops to fgraph_array after
ftrace_startup_subops(). Thus, until the new fgraph_ops is initialized,
we will see fgraph_stub on the corresponding fgraph_array entry.

Cc: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Cc: Florent Revest <revest@chromium.org>
Cc: Martin KaFai Lau <martin.lau@linux.dev>
Cc: bpf <bpf@vger.kernel.org>
Cc: Sven Schnelle <svens@linux.ibm.com>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Alan Maguire <alan.maguire@oracle.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Guo Ren <guoren@kernel.org>
Link: https://lore.kernel.org/172398528350.293426.8347220120333730248.stgit@devnote2
Fixes: c132be2c4f ("function_graph: Have the instances use their own ftrace_ops for filtering")
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
This commit is contained in:
Masami Hiramatsu (Google) 2024-08-18 21:48:03 +09:00 committed by Steven Rostedt (Google)
parent 47ac09b91b
commit a069a22f39
1 changed files with 18 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

31
kernel/trace/fgraph.c
View File

@ -1206,18 +1206,24 @@ static void init_task_vars(int idx)
read_unlock(&tasklist_lock);
}
static void ftrace_graph_enable_direct(bool enable_branch)
static void ftrace_graph_enable_direct(bool enable_branch, struct fgraph_ops *gops)
{
trace_func_graph_ent_t func = NULL;
trace_func_graph_ret_t retfunc = NULL;
int i;
for_each_set_bit(i, &fgraph_array_bitmask,
sizeof(fgraph_array_bitmask) * BITS_PER_BYTE) {
func = fgraph_array[i]->entryfunc;
retfunc = fgraph_array[i]->retfunc;
fgraph_direct_gops = fgraph_array[i];
}
if (gops) {
func = gops->entryfunc;
retfunc = gops->retfunc;
fgraph_direct_gops = gops;
} else {
for_each_set_bit(i, &fgraph_array_bitmask,
sizeof(fgraph_array_bitmask) * BITS_PER_BYTE) {
func = fgraph_array[i]->entryfunc;
retfunc = fgraph_array[i]->retfunc;
fgraph_direct_gops = fgraph_array[i];
}
}
if (WARN_ON_ONCE(!func))
return;
@ -1256,8 +1262,6 @@ int register_ftrace_graph(struct fgraph_ops *gops)
ret = -ENOSPC;
goto out;
}
fgraph_array[i] = gops;
gops->idx = i;
ftrace_graph_active++;
@ -1266,7 +1270,7 @@ int register_ftrace_graph(struct fgraph_ops *gops)
ftrace_graph_disable_direct(true);
if (ftrace_graph_active == 1) {
ftrace_graph_enable_direct(false);
ftrace_graph_enable_direct(false, gops);
register_pm_notifier(&ftrace_suspend_notifier);
ret = start_graph_tracing();
if (ret)
@ -1281,14 +1285,15 @@ int register_ftrace_graph(struct fgraph_ops *gops)
} else {
init_task_vars(gops->idx);
}
/* Always save the function, and reset at unregistering */
gops->saved_func = gops->entryfunc;
ret = ftrace_startup_subops(&graph_ops, &gops->ops, command);
if (!ret)
fgraph_array[i] = gops;
error:
if (ret) {
fgraph_array[i] = &fgraph_stub;
ftrace_graph_active--;
gops->saved_func = NULL;
fgraph_lru_release_index(i);
@ -1324,7 +1329,7 @@ void unregister_ftrace_graph(struct fgraph_ops *gops)
ftrace_shutdown_subops(&graph_ops, &gops->ops, command);
if (ftrace_graph_active == 1)
ftrace_graph_enable_direct(true);
ftrace_graph_enable_direct(true, NULL);
else if (!ftrace_graph_active)
ftrace_graph_disable_direct(false);