Re: [PATCH v7 6/9] perf c2c: add function view hierarchy entry creation

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Manual allocation of `hist_entry` with 0 callchain size causes an out-of-bounds heap read and arbitrary free during `hist_entry__delete()` if callchains are globally enabled.
--

--- Patch [6]: [PATCH v7 6/9] perf c2c: add function view hierarchy entry creation ---
commit 6f1d1721493d4fdd45959567a8ed7813681126d5
Author: Jiebin Sun <[email protected]>

perf c2c: add function view hierarchy entry creation

This commit adds the entry-creation layer for the perf c2c function view. It
introduces owned-reference child allocation and insertion, reusing the existing
cacheline_idx to preserve source entry identity for the browser.

> diff --git a/tools/perf/ui/browsers/c2c-function.c b/tools/perf/ui/browsers/c2c-function.c
> index fc9a4c4bcb03f..fb85f9aeb7415 100644
> --- a/tools/perf/ui/browsers/c2c-function.c
> +++ b/tools/perf/ui/browsers/c2c-function.c

[ ... ]

> @@ -845,6 +863,259 @@ static int __maybe_unused c2c_he__prune_empty_writers(struct hist_entry *l1_he)

[ ... ]

> +static struct c2c_hist_entry *
> +c2c_child_entry__alloc(struct hist_entry *parent_he, struct hist_entry *src_he,
> +		       int depth, u64 ip)
> +{
> +	struct c2c_hist_entry *child_c2c;
> +	struct hist_entry *child_he;
> +
> +	/* Function-view children never own or display callchains. */
> +	child_he = c2c_function_he_zalloc(0);

[Severity: Medium]
Does this allocation cause an out-of-bounds heap read and arbitrary free
during teardown if callchains are globally enabled?

Looking at hist_entry__delete() in tools/perf/util/hist.c, it unconditionally
calls free_callchain(he->callchain) when destroying an entry:

void hist_entry__delete(struct hist_entry *he)
{
	...
	free_callchain(he->callchain);
	...
}

The free_callchain() function checks the global symbol_conf.use_callchain flag
rather than verifying if this specific entry actually allocated a callchain.

Since 0 extra bytes are allocated for the trailing flexible array here, if
callchains are globally enabled (e.g., running perf c2c record -g), wouldn't
the teardown process read unallocated memory, interpret it as nodes in
free_callchain_node(), and trigger memory corruption?

> +	if (!child_he)
> +		return NULL;
> +
> +	child_c2c = container_of(child_he, struct c2c_hist_entry, he);
> +	child_he->ops = &c2c_function_entry_ops;

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=6