Re: [PATCH perf-tools-next v4 3/5] perf ftrace: Support display of inlined functions in function graph tracer

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] perf: buffer overflow in make_blank_prefix() due to unclamped loop bounds
- [Medium] perf ftrace: parser stack desynchronization on FTRACE_MAX_STACK saturation
- [Low] perf/ftrace: stale real frames prevent inactive inlines from being popped
--

commit 74f9ce28f357fc14d2ef1925d4b8504c439b4901
Author: Aaron Tomlin <atomlin@atomlin.com>

perf ftrace: Support display of inlined functions in function graph tracer

This commit introduces the --inline option to perf ftrace for the
function_graph tracer. It leverages DWARF debug information from the
kernel image to resolve return addresses to their inlined callchains and
synthesizes intermediate frames directly into the streamed call graph.

diff --git a/tools/perf/util/ftrace.c b/tools/perf/util/ftrace.c
--- a/tools/perf/util/ftrace.c
+++ b/tools/perf/util/ftrace.c
[ ... ]
> +static void make_blank_prefix(const struct perf_ftrace *ftrace, const char *line,
> +			      const char *pipe_char, char *buf, size_t size)
> +{
> +	size_t len = pipe_char - line + 1;
> +	const char *start_delim = NULL;
> +	char *c;
> +
> +	if (len >= size)
> +		len = size - 1;
> +
> +	memcpy(buf, line, len);
> +	buf[len] = '\0';
> +
> +	if (*pipe_char == '|') {
[ ... ]
> +		if (start_delim && start_delim < pipe_char) {
> +			for (c = buf + (start_delim - line) + 1; c < buf + (pipe_char - line); c++)
> +				*c = ' ';

[Severity: Medium]
Does this code overflow the buf array?

The buf array is populated from a fixed 128-byte array (blank_prefix) in the
caller. The earlier memcpy is safely clamped to size - 1, but this loop bound
uses the unclamped distance pipe_char - line.

If a trace line has an exceptionally long prefix (such as from a long comm
string or malformed input), could pipe_char - line exceed the buffer size and
cause out-of-bounds writes on the stack?

> +		}
> +	}
> +}
[ ... ]
> +int ftrace_process_fgraph_line(struct perf_ftrace *ftrace, const char *line, FILE *out)
> +{
[ ... ]
> +	if (is_exit) {
> +		char exit_name[128];
> +
> +		extract_exit_func_name(func_start, exit_name, sizeof(exit_name));
> +
> +		/* Find matching frame on stack */
> +		match_idx = -1;
> +		for (i = cs->depth - 1; i >= 0; i--) {
> +			if (!cs->stack[i].inlined) {
> +				if (exit_name[0] == '\0' ||
> +				    (cs->stack[i].name &&
> +				     strcmp(cs->stack[i].name, exit_name) == 0)) {
> +					match_idx = i;
> +					break;
> +				}
> +			}
> +		}

[Severity: Medium]
Can this cause parser stack desynchronization on deep call stacks?

If a trace contains more than FTRACE_MAX_STACK (256) nested frames, the
push logic correctly drops them to prevent overflowing the stack tracking.
However, when these untracked frames exit, their trace lines often contain
bare closing braces, which results in exit_name[0] == '\0'.

Will this unconditionally match the topmost valid non-inlined frame on the
stack and pop it prematurely, permanently desynchronizing the stack state
and breaking inline resolution for subsequent callers?

[ ... ]
> +	/* Close inlined frames that are no longer active */
> +	while (active_inlines > common && cs->depth > 0) {
> +		if (cs->stack[cs->depth - 1].inlined) {
> +			struct ftrace_stack_frame *top = &cs->stack[cs->depth - 1];
> +			int exit_indent = top->indent;
> +
> +			if (cs->inlined_depth > 0)
> +				cs->inlined_depth--;
> +			fprintf(out, "%s%*s} /* %s (inline) */\n",
> +				blank_prefix, exit_indent, "", top->name);
> +			free(top->name);
> +			cs->depth--;
> +			active_inlines--;
> +		} else {
> +			break;
> +		}
> +	}

[Severity: Low]
Can stale real frames prevent inactive inlined frames from being popped?

If a function exit event is dropped (for example, due to a ring buffer
overwrite), the corresponding real frame remains on the stack. When the next
entry event is processed and we attempt to close inactive inlined frames above
the caller, the loop breaks immediately upon encountering this stale real frame.

Does this leave the inactive inlines stuck on the stack, corrupting the
nesting and indentation of subsequent call graph output?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261014161457.373253-1-atomlin@atomlin.com?part=3