From: "Masami Hiramatsu (Google)" <mhiramat@kernel.org>
To: Alexei Starovoitov <alexei.starovoitov@gmail.com>,
Steven Rostedt <rostedt@goodmis.org>,
Florent Revest <revest@chromium.org>
Cc: linux-trace-kernel@vger.kernel.org,
LKML <linux-kernel@vger.kernel.org>,
Martin KaFai Lau <martin.lau@linux.dev>,
bpf <bpf@vger.kernel.org>, Sven Schnelle <svens@linux.ibm.com>,
Alexei Starovoitov <ast@kernel.org>, Jiri Olsa <jolsa@kernel.org>,
Arnaldo Carvalho de Melo <acme@kernel.org>,
Daniel Borkmann <daniel@iogearbox.net>,
Alan Maguire <alan.maguire@oracle.com>,
Mark Rutland <mark.rutland@arm.com>,
Peter Zijlstra <peterz@infradead.org>,
Thomas Gleixner <tglx@linutronix.de>, Guo Ren <guoren@kernel.org>
Subject: [PATCH v13 01/20] tracing: fgraph: Fix to add new fgraph_ops to array after ftrace_startup_subops()
Date: Sun, 18 Aug 2024 21:48:03 +0900 [thread overview]
Message-ID: <172398528350.293426.8347220120333730248.stgit@devnote2> (raw)
In-Reply-To: <172398527264.293426.2050093948411376857.stgit@devnote2>
From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
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.
Fixes: c132be2c4fcc ("function_graph: Have the instances use their own ftrace_ops for filtering")
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
kernel/trace/fgraph.c | 31 ++++++++++++++++++-------------
1 file changed, 18 insertions(+), 13 deletions(-)
diff --git a/kernel/trace/fgraph.c b/kernel/trace/fgraph.c
index d1d5ea2d0a1b..d7d4fb403f6f 100644
--- a/kernel/trace/fgraph.c
+++ b/kernel/trace/fgraph.c
@@ -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);
next prev parent reply other threads:[~2024-08-18 12:48 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-08-18 12:47 [PATCH v13 00/20] tracing: fprobe: function_graph: Multi-function graph and fprobe on fgraph Masami Hiramatsu (Google)
2024-08-18 12:48 ` Masami Hiramatsu (Google) [this message]
2024-08-18 12:48 ` [PATCH v13 02/20] tracing: Add a comment about ftrace_regs definition Masami Hiramatsu (Google)
2024-08-18 12:48 ` [PATCH v13 03/20] tracing: Rename ftrace_regs_return_value to ftrace_regs_get_return_value Masami Hiramatsu (Google)
2024-08-18 12:48 ` [PATCH v13 04/20] function_graph: Pass ftrace_regs to entryfunc Masami Hiramatsu (Google)
2024-08-18 12:48 ` [PATCH v13 05/20] function_graph: Replace fgraph_ret_regs with ftrace_regs Masami Hiramatsu (Google)
2024-08-18 12:48 ` [PATCH v13 06/20] function_graph: Pass ftrace_regs to retfunc Masami Hiramatsu (Google)
2024-08-18 12:49 ` [PATCH v13 07/20] fprobe: Use ftrace_regs in fprobe entry handler Masami Hiramatsu (Google)
2024-08-18 12:49 ` [PATCH v13 08/20] fprobe: Use ftrace_regs in fprobe exit handler Masami Hiramatsu (Google)
2024-08-18 12:49 ` [PATCH v13 09/20] tracing: Add ftrace_partial_regs() for converting ftrace_regs to pt_regs Masami Hiramatsu (Google)
2024-08-18 12:49 ` [PATCH v13 10/20] tracing: Add ftrace_fill_perf_regs() for perf event Masami Hiramatsu (Google)
2024-08-18 12:49 ` [PATCH v13 11/20] tracing/fprobe: Enable fprobe events with CONFIG_DYNAMIC_FTRACE_WITH_ARGS Masami Hiramatsu (Google)
2024-08-18 12:49 ` [PATCH v13 12/20] bpf: Enable kprobe_multi feature if CONFIG_FPROBE is enabled Masami Hiramatsu (Google)
2024-08-18 12:50 ` [PATCH v13 13/20] ftrace: Add CONFIG_HAVE_FTRACE_GRAPH_FUNC Masami Hiramatsu (Google)
2024-08-18 12:50 ` [PATCH v13 14/20] fprobe: Rewrite fprobe on function-graph tracer Masami Hiramatsu (Google)
2024-08-18 12:50 ` [PATCH v13 15/20] tracing: Fix function timing profiler to initialize hashtable Masami Hiramatsu (Google)
2024-08-18 12:50 ` [PATCH v13 16/20] tracing/fprobe: Remove nr_maxactive from fprobe Masami Hiramatsu (Google)
2024-08-18 12:50 ` [PATCH v13 17/20] selftests: ftrace: Remove obsolate maxactive syntax check Masami Hiramatsu (Google)
2024-08-18 12:51 ` [PATCH v13 18/20] selftests/ftrace: Add a test case for repeating register/unregister fprobe Masami Hiramatsu (Google)
2024-08-18 12:51 ` [PATCH v13 19/20] Documentation: probes: Update fprobe on function-graph tracer Masami Hiramatsu (Google)
2024-08-18 12:51 ` [PATCH v13 20/20] fgraph: Skip recording calltime/rettime if it is not nneeded Masami Hiramatsu (Google)
2024-09-11 0:24 ` [PATCH v13 00/20] tracing: fprobe: function_graph: Multi-function graph and fprobe on fgraph Masami Hiramatsu
2024-09-11 23:49 ` Masami Hiramatsu
2024-09-11 18:27 ` Jiri Olsa
2024-09-12 1:09 ` Masami Hiramatsu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=172398528350.293426.8347220120333730248.stgit@devnote2 \
--to=mhiramat@kernel.org \
--cc=acme@kernel.org \
--cc=alan.maguire@oracle.com \
--cc=alexei.starovoitov@gmail.com \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=guoren@kernel.org \
--cc=jolsa@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-trace-kernel@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=martin.lau@linux.dev \
--cc=peterz@infradead.org \
--cc=revest@chromium.org \
--cc=rostedt@goodmis.org \
--cc=svens@linux.ibm.com \
--cc=tglx@linutronix.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).