Re: [PATCH perf-tools-next v6 5/5] perf ftrace: Filter extraneous 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/ftrace: target filter state leak if target root is an overflowed frame
- [Medium] perf/ftrace: name collision in orphan exit corrupts target tracking stack
- [Medium] perf/ftrace: orphan call filter bypassed when stack is overflowed
--

commit 507b405f29e53ab24cb9856a78d324efaeccdfc0
Author: Aaron Tomlin <atomlin@atomlin.com>

perf ftrace: Filter extraneous functions in function graph tracer

This commit filters extraneous functions (such as transient top-level
functions or detached orphan calls inside the target callgraph)
deterministically in user space when using the function graph tracer with
a targeted filter.

> diff --git a/tools/perf/util/ftrace.c b/tools/perf/util/ftrace.c
> index 6cdb87b445eaa..8a5d2bdf07207 100644
> --- a/tools/perf/util/ftrace.c
> +++ b/tools/perf/util/ftrace.c

[ ... ]

> @@ -477,30 +541,57 @@ int ftrace_process_fgraph_line(struct perf_ftrace *ftrace, const char *line, FIL
>  	if (is_exit) {
>  		char exit_name[128];
>  
>  		extract_exit_func_name(func_start, exit_name, sizeof(exit_name));
>  
> -		/* Find matching frame on stack */
> +		/* Find matching frame on stack if exit is named */
>  		match_idx = -1;
> +		if (exit_name[0] != '\0') {
> +			for (i = cs->depth - 1; i >= 0; i--) {
> +				if (!cs->stack[i].inlined && cs->stack[i].name &&
> +				    strcmp(cs->stack[i].name, exit_name) == 0) {
>  					match_idx = i;
>  					break;
>  				}
>  			}
>  		}
>  
> +		if (match_idx >= 0) {
> +			/* Named exit matches a tracked frame; clear stale orphan depth */
> +			cs->orphan_depth = 0;
> +		} else if (cs->orphan_depth > 0) {
> +			cs->orphan_depth--;
> +			return 0;
> +		}

[Severity: Medium]
Will this unconditional scan of the tracking stack cause name collisions if
a dropped orphan exit has the same name as an active frame?

The parser clears the orphan state and pops the stack if a name match is found,
blindly trusting that the named exit belongs to the tracked stack rather than
the currently active orphan hierarchy.

If an unrelated orphan call shares the same name as a legitimate frame that
is currently active on the main target stack, it will prematurely match that
frame. The parser incorrectly assumes the tracked target function has exited,
clears orphan_depth, and unspools the tracking stack up to that point.

Could this corrupt the target filter state and cause the actual target
function's eventual exit to be mishandled?

> +
> +		if (cs->overflow_depth > 0) {
> +			cs->overflow_depth--;
> +			if (!has_filter || cs->in_target) {
> +				if (!ftrace->use_inline) {
> +					print_line(ftrace, line, out);
> +				} else {
> +					extra_indent = cs->inlined_depth * 2;
> +					fprintf(out, "%.*s%*s%s\n",
> +						(int)(pipe_char - line + 1), line,
> +						leading_spaces + extra_indent, "", func_start);
> +				}
> +			}
> +			return 0;
> +		}

[Severity: Medium]
Does this early return skip the filter teardown logic located at the bottom
of the is_exit block?

If a target function is matched when the stack is already at FTRACE_MAX_STACK,
in_target is set to true and target_root_depth is assigned FTRACE_MAX_STACK.
The parser enters the function, incrementing overflow_depth to 1 instead of
pushing a new tracking frame.

Upon the function's exit, the parser decrements overflow_depth and returns
early from the is_exit block. Because the cs->in_target = false teardown
logic is skipped, any subsequent unrelated sibling functions executing at that
stack depth will be leaked into the filtered trace output until the parent
frame exits and cs->depth drops below the maximum stack limit.

[ ... ]

> @@ -566,6 +824,22 @@ int ftrace_process_fgraph_line(struct perf_ftrace *ftrace, const char *line, FIL
>  		}
>  	}
>  
> +	/*
> +	 * When inside a target graph filter, child functions must descend
> +	 * from an active frame on the target stack. Suppress detached
> +	 * calls originating outside the target hierarchy.
> +	 */
> +	if (has_filter && cs->in_target && cs->depth > cs->target_root_depth) {
> +		if (has_retaddr && (caller_idx >= 0 ? caller_idx < cs->target_root_depth
> +						    : cs->overflow_depth == 0)) {
> +			if (has_open_brace && !has_semicolon)
> +				cs->orphan_depth = 1;
> +			for (i = 0; i < num_inlines; i++)
> +				free((void *)inlined_names[i]);
> +			return 0;
> +		}
> +	}

[Severity: Medium]
When the stack is overflowed (overflow_depth > 0), does this fallback
condition evaluate to false and bypass the orphan filter?

To suppress detached orphan calls originating outside the target hierarchy,
the parser verifies that each child entry descends from an active target stack
frame by resolving its return address. If the caller is not found on the active
tracking stack (caller_idx < 0), the code checks if cs->overflow_depth == 0.

If the stack is actively overflowed (overflow_depth > 0), the condition
evaluates to false, causing the parser to not return early to suppress the
call. The logic blindly assumes that any unknown caller during an overflow
must be a legitimate overflowed child.

Could this disable the orphan suppression mechanism during deep call stacks,
allowing unrelated interrupt handlers or rb_wakeups() subtrees to leak into
the filtered target output?

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