The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [for-next][PATCH 0/3] ftrace: Updates for v6.15
@ 2025-03-24 21:18 Steven Rostedt
  2025-03-24 21:18 ` [for-next][PATCH 1/3] function_graph: Remove the unused variable func Steven Rostedt
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Steven Rostedt @ 2025-03-24 21:18 UTC (permalink / raw)
  To: linux-kernel
  Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Andrew Morton

  git://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace.git
ftrace/for-next

Head SHA1: 391dda1bd7c56de62b96126214f040fe8965561b


Jiapeng Chong (1):
      function_graph: Remove the unused variable func

Sasha Levin (1):
      tracing: Use hashtable.h for event_hash

Tengda Wu (1):
      tracing: Fix use-after-free in print_graph_function_flags during tracer switching

----
 kernel/trace/trace_functions_graph.c |  4 +---
 kernel/trace/trace_irqsoff.c         |  2 --
 kernel/trace/trace_output.c          | 19 +++++++------------
 kernel/trace/trace_sched_wakeup.c    |  2 --
 4 files changed, 8 insertions(+), 19 deletions(-)

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [for-next][PATCH 1/3] function_graph: Remove the unused variable func
  2025-03-24 21:18 [for-next][PATCH 0/3] ftrace: Updates for v6.15 Steven Rostedt
@ 2025-03-24 21:18 ` Steven Rostedt
  2025-03-24 21:18 ` [for-next][PATCH 2/3] tracing: Fix use-after-free in print_graph_function_flags during tracer switching Steven Rostedt
  2025-03-24 21:18 ` [for-next][PATCH 3/3] tracing: Use hashtable.h for event_hash Steven Rostedt
  2 siblings, 0 replies; 4+ messages in thread
From: Steven Rostedt @ 2025-03-24 21:18 UTC (permalink / raw)
  To: linux-kernel
  Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Andrew Morton,
	Abaci Robot, Jiapeng Chong

From: Jiapeng Chong <jiapeng.chong@linux.alibaba.com>

Variable func is not effectively used, so delete it.

kernel/trace/trace_functions_graph.c:925:16: warning: variable ‘func’ set but not used.

This happened because the variable "func" which came from "call->func" was
replaced by "ret_func" coming from "graph_ret->func" but "func" wasn't
removed after the replacement.

Link: https://lore.kernel.org/20250307021412.119107-1-jiapeng.chong@linux.alibaba.com
Reported-by: Abaci Robot <abaci@linux.alibaba.com>
Closes: https://bugzilla.openanolis.cn/show_bug.cgi?id=19250
Fixes: ff5c9c576e754 ("ftrace: Add support for function argument to graph tracer")
Signed-off-by: Jiapeng Chong <jiapeng.chong@linux.alibaba.com>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 kernel/trace/trace_functions_graph.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/kernel/trace/trace_functions_graph.c b/kernel/trace/trace_functions_graph.c
index 71b2fb068b6b..ed61ff719aa4 100644
--- a/kernel/trace/trace_functions_graph.c
+++ b/kernel/trace/trace_functions_graph.c
@@ -922,7 +922,6 @@ print_graph_entry_leaf(struct trace_iterator *iter,
 	struct ftrace_graph_ent *call;
 	unsigned long long duration;
 	unsigned long ret_func;
-	unsigned long func;
 	int args_size;
 	int cpu = iter->cpu;
 	int i;
@@ -933,8 +932,6 @@ print_graph_entry_leaf(struct trace_iterator *iter,
 	call = &entry->graph_ent;
 	duration = ret_entry->rettime - ret_entry->calltime;
 
-	func = call->func + iter->tr->text_delta;
-
 	if (data) {
 		struct fgraph_cpu_data *cpu_data;
 
-- 
2.47.2



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [for-next][PATCH 2/3] tracing: Fix use-after-free in print_graph_function_flags during tracer switching
  2025-03-24 21:18 [for-next][PATCH 0/3] ftrace: Updates for v6.15 Steven Rostedt
  2025-03-24 21:18 ` [for-next][PATCH 1/3] function_graph: Remove the unused variable func Steven Rostedt
@ 2025-03-24 21:18 ` Steven Rostedt
  2025-03-24 21:18 ` [for-next][PATCH 3/3] tracing: Use hashtable.h for event_hash Steven Rostedt
  2 siblings, 0 replies; 4+ messages in thread
From: Steven Rostedt @ 2025-03-24 21:18 UTC (permalink / raw)
  To: linux-kernel
  Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Andrew Morton,
	stable, Zheng Yejian, Kairui Song, Tengda Wu

From: Tengda Wu <wutengda@huaweicloud.com>

Kairui reported a UAF issue in print_graph_function_flags() during
ftrace stress testing [1]. This issue can be reproduced if puting a
'mdelay(10)' after 'mutex_unlock(&trace_types_lock)' in s_start(),
and executing the following script:

  $ echo function_graph > current_tracer
  $ cat trace > /dev/null &
  $ sleep 5  # Ensure the 'cat' reaches the 'mdelay(10)' point
  $ echo timerlat > current_tracer

The root cause lies in the two calls to print_graph_function_flags
within print_trace_line during each s_show():

  * One through 'iter->trace->print_line()';
  * Another through 'event->funcs->trace()', which is hidden in
    print_trace_fmt() before print_trace_line returns.

Tracer switching only updates the former, while the latter continues
to use the print_line function of the old tracer, which in the script
above is print_graph_function_flags.

Moreover, when switching from the 'function_graph' tracer to the
'timerlat' tracer, s_start only calls graph_trace_close of the
'function_graph' tracer to free 'iter->private', but does not set
it to NULL. This provides an opportunity for 'event->funcs->trace()'
to use an invalid 'iter->private'.

To fix this issue, set 'iter->private' to NULL immediately after
freeing it in graph_trace_close(), ensuring that an invalid pointer
is not passed to other tracers. Additionally, clean up the unnecessary
'iter->private = NULL' during each 'cat trace' when using wakeup and
irqsoff tracers.

 [1] https://lore.kernel.org/all/20231112150030.84609-1-ryncsn@gmail.com/

Cc: stable@vger.kernel.org
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Zheng Yejian <zhengyejian1@huawei.com>
Link: https://lore.kernel.org/20250320122137.23635-1-wutengda@huaweicloud.com
Fixes: eecb91b9f98d ("tracing: Fix memleak due to race between current_tracer and trace")
Closes: https://lore.kernel.org/all/CAMgjq7BW79KDSCyp+tZHjShSzHsScSiJxn5ffskp-QzVM06fxw@mail.gmail.com/
Reported-by: Kairui Song <kasong@tencent.com>
Signed-off-by: Tengda Wu <wutengda@huaweicloud.com>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 kernel/trace/trace_functions_graph.c | 1 +
 kernel/trace/trace_irqsoff.c         | 2 --
 kernel/trace/trace_sched_wakeup.c    | 2 --
 3 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/kernel/trace/trace_functions_graph.c b/kernel/trace/trace_functions_graph.c
index ed61ff719aa4..2f077d4158e5 100644
--- a/kernel/trace/trace_functions_graph.c
+++ b/kernel/trace/trace_functions_graph.c
@@ -1611,6 +1611,7 @@ void graph_trace_close(struct trace_iterator *iter)
 	if (data) {
 		free_percpu(data->cpu_data);
 		kfree(data);
+		iter->private = NULL;
 	}
 }
 
diff --git a/kernel/trace/trace_irqsoff.c b/kernel/trace/trace_irqsoff.c
index c8bfa7310a91..40c39e946940 100644
--- a/kernel/trace/trace_irqsoff.c
+++ b/kernel/trace/trace_irqsoff.c
@@ -250,8 +250,6 @@ static void irqsoff_trace_open(struct trace_iterator *iter)
 {
 	if (is_graph(iter->tr))
 		graph_trace_open(iter);
-	else
-		iter->private = NULL;
 }
 
 static void irqsoff_trace_close(struct trace_iterator *iter)
diff --git a/kernel/trace/trace_sched_wakeup.c b/kernel/trace/trace_sched_wakeup.c
index c9ba4259e03e..a0db3404f7f7 100644
--- a/kernel/trace/trace_sched_wakeup.c
+++ b/kernel/trace/trace_sched_wakeup.c
@@ -188,8 +188,6 @@ static void wakeup_trace_open(struct trace_iterator *iter)
 {
 	if (is_graph(iter->tr))
 		graph_trace_open(iter);
-	else
-		iter->private = NULL;
 }
 
 static void wakeup_trace_close(struct trace_iterator *iter)
-- 
2.47.2



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [for-next][PATCH 3/3] tracing: Use hashtable.h for event_hash
  2025-03-24 21:18 [for-next][PATCH 0/3] ftrace: Updates for v6.15 Steven Rostedt
  2025-03-24 21:18 ` [for-next][PATCH 1/3] function_graph: Remove the unused variable func Steven Rostedt
  2025-03-24 21:18 ` [for-next][PATCH 2/3] tracing: Fix use-after-free in print_graph_function_flags during tracer switching Steven Rostedt
@ 2025-03-24 21:18 ` Steven Rostedt
  2 siblings, 0 replies; 4+ messages in thread
From: Steven Rostedt @ 2025-03-24 21:18 UTC (permalink / raw)
  To: linux-kernel
  Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Andrew Morton,
	Sasha Levin

From: Sasha Levin <sashal@kernel.org>

Convert the event_hash array in trace_output.c to use the generic
hashtable implementation from hashtable.h instead of the manually
implemented hash table.

This simplifies the code and makes it more maintainable by using the
standard hashtable API defined in hashtable.h.

Rename EVENT_HASHSIZE to EVENT_HASH_BITS to properly reflect its new
meaning as the number of bits for the hashtable size.

Link: https://lore.kernel.org/20250323132800.3010783-1-sashal@kernel.org
Link: https://lore.kernel.org/20250319190545.3058319-1-sashal@kernel.org
Signed-off-by: Sasha Levin <sashal@kernel.org>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 kernel/trace/trace_output.c | 19 +++++++------------
 1 file changed, 7 insertions(+), 12 deletions(-)

diff --git a/kernel/trace/trace_output.c b/kernel/trace/trace_output.c
index b51ee9373773..72b699f909e8 100644
--- a/kernel/trace/trace_output.c
+++ b/kernel/trace/trace_output.c
@@ -14,16 +14,17 @@
 #include <linux/idr.h>
 #include <linux/btf.h>
 #include <linux/bpf.h>
+#include <linux/hashtable.h>
 
 #include "trace_output.h"
 #include "trace_btf.h"
 
-/* must be a power of 2 */
-#define EVENT_HASHSIZE	128
+/* 2^7 = 128 */
+#define EVENT_HASH_BITS 7
 
 DECLARE_RWSEM(trace_event_sem);
 
-static struct hlist_head event_hash[EVENT_HASHSIZE] __read_mostly;
+static DEFINE_HASHTABLE(event_hash, EVENT_HASH_BITS);
 
 enum print_line_t trace_print_bputs_msg_only(struct trace_iterator *iter)
 {
@@ -779,11 +780,8 @@ void print_function_args(struct trace_seq *s, unsigned long *args,
 struct trace_event *ftrace_find_event(int type)
 {
 	struct trace_event *event;
-	unsigned key;
 
-	key = type & (EVENT_HASHSIZE - 1);
-
-	hlist_for_each_entry(event, &event_hash[key], node) {
+	hash_for_each_possible(event_hash, event, node, type) {
 		if (event->type == type)
 			return event;
 	}
@@ -838,7 +836,6 @@ void trace_event_read_unlock(void)
  */
 int register_trace_event(struct trace_event *event)
 {
-	unsigned key;
 	int ret = 0;
 
 	down_write(&trace_event_sem);
@@ -871,9 +868,7 @@ int register_trace_event(struct trace_event *event)
 	if (event->funcs->binary == NULL)
 		event->funcs->binary = trace_nop_print;
 
-	key = event->type & (EVENT_HASHSIZE - 1);
-
-	hlist_add_head(&event->node, &event_hash[key]);
+	hash_add(event_hash, &event->node, event->type);
 
 	ret = event->type;
  out:
@@ -888,7 +883,7 @@ EXPORT_SYMBOL_GPL(register_trace_event);
  */
 int __unregister_trace_event(struct trace_event *event)
 {
-	hlist_del(&event->node);
+	hash_del(&event->node);
 	free_trace_event_type(event->type);
 	return 0;
 }
-- 
2.47.2



^ permalink raw reply related	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2025-03-24 21:17 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-24 21:18 [for-next][PATCH 0/3] ftrace: Updates for v6.15 Steven Rostedt
2025-03-24 21:18 ` [for-next][PATCH 1/3] function_graph: Remove the unused variable func Steven Rostedt
2025-03-24 21:18 ` [for-next][PATCH 2/3] tracing: Fix use-after-free in print_graph_function_flags during tracer switching Steven Rostedt
2025-03-24 21:18 ` [for-next][PATCH 3/3] tracing: Use hashtable.h for event_hash Steven Rostedt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox