On Fri, 8 Aug 2025 22:24:05 -0400
Sasha Levin <sashal@xxxxxxxxxx> wrote:
> So we've added a dynamically sized array to the end of
> ftrace_graph_ent_entry, but in struct fgraph_data, the saved entry is
> defined as:
>
> struct fgraph_data {
> ...
> union {
> struct ftrace_graph_ent_entry ent;
> struct fgraph_retaddr_ent_entry rent;
> } ent;
> ...
> }
>
> Which doesn't seem to have room for args?
No it doesn't :-p
>
> The code in get_return_for_leaf() does:
>
> data->ent.ent = *curr;
>
> This copies the struct, but curr points to a larger entry with args
> data. The copy operation only copies sizeof(struct
> ftrace_graph_ent_entry) bytes, which doesn't include the dynamic args
> array.
>
> And then later functions (like print_graph_entry()) would go ahead and
> assume that iter->ent_size is sane and make a mess out of everything.
>
> I can't test right now whether this actually fixes the issues or not,
> but I wanted to bring this up as this looks somewhat odd and I'm not too
> familiar with this code.
Thanks for the detail analysis, can you test this patch?
-- Steve
diff --git a/kernel/trace/trace_functions_graph.c b/kernel/trace/trace_functions_graph.c
index 66e1a527cf1a..25ea71edb8da 100644
--- a/kernel/trace/trace_functions_graph.c
+++ b/kernel/trace/trace_functions_graph.c
@@ -35,6 +35,11 @@ struct fgraph_data {
struct ftrace_graph_ent_entry ent;
struct fgraph_retaddr_ent_entry rent;
} ent;
+ /*
+ * The @args must be right after @ent, as it is where they
+ * are stored in case the function graph tracer has arguments.
+ */
+ unsigned long args[FTRACE_REGS_MAX_ARGS];
struct ftrace_graph_ret_entry ret;
int failed;
int cpu;
@@ -623,14 +628,29 @@ get_return_for_leaf(struct trace_iterator *iter,
next = ring_buffer_event_data(event);
if (data) {
+ int args_size;
+ int size;
+
/*
* Save current and next entries for later reference
* if the output fails.
*/
- if (unlikely(curr->ent.type == TRACE_GRAPH_RETADDR_ENT))
+ if (unlikely(curr->ent.type == TRACE_GRAPH_RETADDR_ENT)) {
data->ent.rent = *(struct fgraph_retaddr_ent_entry *)curr;
- else
+ size = offsetof(struct fgraph_retaddr_ent_entry, args);
+ } else {
data->ent.ent = *curr;
+ size = offsetof(struct ftrace_graph_ent_entry, args);
+ }
+
+ /* If this has args, then append them to after the ent. */
+ args_size = iter->ent_size - size;
+ if (args_size > sizeof(long) * FTRACE_REGS_MAX_ARGS)
+ args_size = sizeof(long) * FTRACE_REGS_MAX_ARGS;
+
+ if (args_size >= sizeof(long))
+ memcpy((void *)&data->ent.ent + size,
+ (void*)curr + size, args_size);
/*
* If the next event is not a return type, then
* we only care about what type it is. Otherwise we can
