linux-kernel - [for-linus][PATCH] fgraph: Copy args in intermediate storage with entry

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [day] [month] [year] [list]
Message-ID: <20250821110723.4395212a@gandalf.local.home>
Date: Thu, 21 Aug 2025 11:07:23 -0400
From: Steven Rostedt <rostedt@...dmis.org>
To: LKML <linux-kernel@...r.kernel.org>
Cc: Masami Hiramatsu <mhiramat@...nel.org>, Mathieu Desnoyers
 <mathieu.desnoyers@...icios.com>, Mark Rutland <mark.rutland@....com>,
 Sasha Levin <sashal@...nel.org>
Subject: [for-linus][PATCH] fgraph: Copy args in intermediate storage with
 entry


  git://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace.git
trace/fixes

Head SHA1: b7b6a20aa4ab811f598793210b4ea62885b40e18


Steven Rostedt (1):
      fgraph: Copy args in intermediate storage with entry

----
 kernel/trace/trace_functions_graph.c | 22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)
---------------------------
commit b7b6a20aa4ab811f598793210b4ea62885b40e18
Author: Steven Rostedt <rostedt@...dmis.org>
Date:   Wed Aug 20 19:55:22 2025 -0400

    fgraph: Copy args in intermediate storage with entry
    
    The output of the function graph tracer has two ways to display its
    entries. One way for leaf functions with no events recorded within them,
    and the other is for functions with events recorded inside it. As function
    graph has an entry and exit event, to simplify the output of leaf
    functions it combines the two, where as non leaf functions are separate:
    
     2)               |              invoke_rcu_core() {
     2)               |                raise_softirq() {
     2)   0.391 us    |                  __raise_softirq_irqoff();
     2)   1.191 us    |                }
     2)   2.086 us    |              }
    
    The __raise_softirq_irqoff() function above is really two events that were
    merged into one. Otherwise it would have looked like:
    
     2)               |              invoke_rcu_core() {
     2)               |                raise_softirq() {
     2)               |                  __raise_softirq_irqoff() {
     2)   0.391 us    |                  }
     2)   1.191 us    |                }
     2)   2.086 us    |              }
    
    In order to do this merge, the reading of the trace output file needs to
    look at the next event before printing. But since the pointer to the event
    is on the ring buffer, it needs to save the entry event before it looks at
    the next event as the next event goes out of focus as soon as a new event
    is read from the ring buffer. After it reads the next event, it will print
    the entry event with either the '{' (non leaf) or ';' and timestamps (leaf).
    
    The iterator used to read the trace file has storage for this event. The
    problem happens when the function graph tracer has arguments attached to
    the entry event as the entry now has a variable length "args" field. This
    field only gets set when funcargs option is used. But the args are not
    recorded in this temp data and garbage could be printed. The entry field
    is copied via:
    
      data->ent = *curr;
    
    Where "curr" is the entry field. But this method only saves the non
    variable length fields from the structure.
    
    Add a helper structure to the iterator data that adds the max args size to
    the data storage in the iterator. Then simply copy the entire entry into
    this storage (with size protection).
    
    Cc: Masami Hiramatsu <mhiramat@...nel.org>
    Cc: Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
    Cc: Mark Rutland <mark.rutland@....com>
    Link: https://lore.kernel.org/20250820195522.51d4a268@gandalf.local.home
    Reported-by: Sasha Levin <sashal@...nel.org>
    Closes: https://lore.kernel.org/all/aJaxRVKverIjF4a6@lappy/
    Fixes: ff5c9c576e75 ("ftrace: Add support for function argument to graph tracer")
    Signed-off-by: Steven Rostedt (Google) <rostedt@...dmis.org>

diff --git a/kernel/trace/trace_functions_graph.c b/kernel/trace/trace_functions_graph.c
index 66e1a527cf1a..a7f4b9a47a71 100644
--- a/kernel/trace/trace_functions_graph.c
+++ b/kernel/trace/trace_functions_graph.c
@@ -27,14 +27,21 @@ struct fgraph_cpu_data {
 	unsigned long	enter_funcs[FTRACE_RETFUNC_DEPTH];
 };
 
+struct fgraph_ent_args {
+	struct ftrace_graph_ent_entry	ent;
+	/* Force the sizeof of args[] to have FTRACE_REGS_MAX_ARGS entries */
+	unsigned long			args[FTRACE_REGS_MAX_ARGS];
+};
+
 struct fgraph_data {
 	struct fgraph_cpu_data __percpu *cpu_data;
 
 	/* Place to preserve last processed entry. */
 	union {
-		struct ftrace_graph_ent_entry	ent;
+		struct fgraph_ent_args		ent;
+		/* TODO allow retaddr to have args */
 		struct fgraph_retaddr_ent_entry	rent;
-	} ent;
+	};
 	struct ftrace_graph_ret_entry	ret;
 	int				failed;
 	int				cpu;
@@ -627,10 +634,13 @@ get_return_for_leaf(struct trace_iterator *iter,
 			 * Save current and next entries for later reference
 			 * if the output fails.
 			 */
-			if (unlikely(curr->ent.type == TRACE_GRAPH_RETADDR_ENT))
-				data->ent.rent = *(struct fgraph_retaddr_ent_entry *)curr;
-			else
-				data->ent.ent = *curr;
+			if (unlikely(curr->ent.type == TRACE_GRAPH_RETADDR_ENT)) {
+				data->rent = *(struct fgraph_retaddr_ent_entry *)curr;
+			} else {
+				int size = min((int)sizeof(data->ent), (int)iter->ent_size);
+
+				memcpy(&data->ent, curr, size);
+			}
 			/*
 			 * If the next event is not a return type, then
 			 * we only care about what type it is. Otherwise we can