Re: [PATCH perf-tools-next v4 5/5] perf ftrace: Filter extraneous functions in function graph tracer

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] perf/ftrace: filter state leak on leaf functions with inlines
- [Medium] perf: stack buffer overflow in make_blank_prefix()
--

commit 5c319821fcd67517723413eff78ea02ad9a706a1
Author: Aaron Tomlin <atomlin@atomlin.com>

perf ftrace: Filter extraneous functions in function graph tracer

This commit introduces a mechanism to track active callgraph state per CPU in
the perf ftrace function graph tracer. It suppresses unrelated top-level
functions and detached orphan calls from the trace output when using a
targeted filter.

> 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

[ ... ]

> @@ -443,8 +479,9 @@ int ftrace_process_fgraph_line(struct perf_ftrace *ftrace, const char *line, FIL
>  	bool has_open_brace;
>  	bool has_semicolon;
>  	bool has_retaddr = false;
> +	bool has_filter = ftrace_has_graph_funcs(ftrace);
>  
> -	if (!ftrace->use_inline)
> +	if (!has_filter && !ftrace->use_inline)
>  		return print_line(ftrace, line, out);
>  
>  	pipe_char = find_func_column(ftrace, line, &cpu);

[Severity: Medium]
Shortly after this in ftrace_process_fgraph_line(), make_blank_prefix() is
called and provided with the 128-byte blank_prefix array from the stack:

    if (start_delim && start_delim < pipe_char) {
        for (c = buf + (start_delim - line) + 1; c < buf + (pipe_char - line); c++)
            *c = ' ';
    }

Can this loop in make_blank_prefix() write out of bounds?

The loop blindly writes space characters up to buf + (pipe_char - line). If
the trace line prefix before pipe_char exceeds 128 characters, it appears
this will write past the end of the blank_prefix stack buffer without
clamping to the size of the destination.

[ ... ]

> @@ -549,6 +691,21 @@ 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 < cs->target_root_depth) {
> +			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]
Because cs->depth is inflated by the inline frames for a leaf target function,
the filter state check later on fails and leaves cs->in_target set to true.

When that happens, does this block cause subsequent sibling functions to
incorrectly return early, entirely skipping the logic that would normally
close the stale inline frames?

[ ... ]

> @@ -659,6 +816,13 @@ int ftrace_process_fgraph_line(struct perf_ftrace *ftrace, const char *line, FIL
>  		}
>  	}
>  
> +	if (has_filter && cs->in_target && cs->depth <= cs->target_root_depth &&
> +	    has_semicolon && !has_open_brace) {
> +		cs->in_target = false;
> +		cs->target_root_depth = 0;
> +		cs->orphan_depth = 0;
> +	}
> +
>  	for (i = 0; i < num_inlines; i++)
>  		free((void *)inlined_names[i]);
>  	return 0;

[Severity: Medium]
Does this reset condition fail for leaf functions when ftrace->use_inline is
true?

If a targeted leaf function resolves to inline frames,
ftrace_process_fgraph_line() pushes the inline frames onto cs->stack and
increments cs->depth. Because cs->depth is inflated by num_inlines, the
check against cs->target_root_depth will be false, leaving in_target
permanently true for leaf calls with inlines.

Could this leak the filter state, causing subsequent sibling functions to be
incorrectly evaluated as being inside the target graph, leading to malformed
or missing trace lines?

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