Commit e3d01979 authored by Steven Rostedt's avatar Steven Rostedt Committed by Steven Rostedt (Google)
Browse files

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@kernel.org>
Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Link: https://lore.kernel.org/20250820195522.51d4a268@gandalf.local.home


Reported-by: default avatarSasha Levin <sashal@kernel.org>
Tested-by: default avatarSasha Levin <sashal@kernel.org>
Closes: https://lore.kernel.org/all/aJaxRVKverIjF4a6@lappy/


Fixes: ff5c9c57 ("ftrace: Add support for function argument to graph tracer")
Signed-off-by: default avatarSteven Rostedt (Google) <rostedt@goodmis.org>
parent edede7a6
Loading
Loading
Loading
Loading
+16 −6
Original line number Diff line number Diff line
@@ -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