Re: [PATCH] fgraph: Copy args in intermediate storage with entry

[Sasha Levin <sashal@kernel.org>]

On Wed, Aug 20, 2025 at 07:55:22PM -0400, Steven Rostedt wrote:
>From: Steven Rostedt <rostedt@goodmis.org>
>
>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).
>
>Reported-by: Sasha Levin <sashal@kernel.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@goodmis.org>

	Tested-by: Sasha Levin <sashal@kernel.org>

Thanks for the fix!

-- 
Thanks,
Sasha