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

On Wed, Aug 12, 2026 at 10:47:17AM +0900, Namhyung Kim wrote:
> Hello,
> 
> On Mon, Aug 10, 2026 at 05:49:44AM +0000, [email protected] wrote:
> > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> > - [Medium] Deleting a function-view child entry causes an out-of-bounds heap read in `free_callchain()` because the child entry is allocated without trailing `callchain_root` memory.
> > --
> > 
> > --- Patch [6]: [PATCH v8 6/9] perf c2c: add function view hierarchy entry creation ---
> > commit 3fbce53bcfd8d8abeda29f9eded35e05dfc305f5
> > Author: Jiebin Sun <[email protected]>
> > 
> > perf c2c: add function view hierarchy entry creation
> >     
> > This patch adds the entry-creation layer for the function view hierarchy.
> > It implements level-1/2/3 lookup-or-create functions and synthetic child
> > allocation logic, reusing cacheline_idx to preserve source entry identity.
> > 
> > > diff --git a/tools/perf/ui/browsers/c2c-function.c b/tools/perf/ui/browsers/c2c-function.c
> > > index 4bf8406fde556..4099d4c7e7b44 100644
> > > --- a/tools/perf/ui/browsers/c2c-function.c
> > > +++ b/tools/perf/ui/browsers/c2c-function.c
> > 
> > [ ... ]
> > 
> > > @@ -847,6 +865,259 @@ static int __maybe_unused c2c_he__prune_empty_writers(struct hist_entry *l1_he)
> > >  	return surviving;
> > >  }
> > >  
> > > +static void *c2c_function_he_zalloc(size_t size)
> > > +{
> > > +	struct c2c_hist_entry *c2c_he = zalloc(sizeof(*c2c_he) + size);
> > > +
> > > +	if (!c2c_he)
> > > +		return NULL;
> > > +
> > > +	init_stats(&c2c_he->cstats.lcl_hitm);
> > > +	init_stats(&c2c_he->cstats.rmt_hitm);
> > > +	init_stats(&c2c_he->cstats.lcl_peer);
> > > +	init_stats(&c2c_he->cstats.rmt_peer);
> > > +	init_stats(&c2c_he->cstats.load);
> > > +
> > > +	return &c2c_he->he;
> > > +}
> > > +
> > > +/* Entry operations for function view */
> > > +static struct hist_entry_ops c2c_function_entry_ops = {
> > > +	.new	= c2c_function_he_zalloc,
> > > +	.free	= c2c_function_he_free,
> > > +};
> > > +
> > > +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);
> > > +	if (!child_he)
> > > +		return NULL;
> > 
> > [Severity: Medium]
> > Will this cause an out-of-bounds memory read when the entry is freed?
> > 
> > If perf c2c report is run with callchains enabled (symbol_conf.use_callchain
> > is true), this entry is allocated with 0 extra bytes for the callchain.
> > 
> > When this undersized entry is deleted on the error path below or during
> > cleanup via c2c_he__free_hierarchy(), hist_entry__delete() accesses
> > he->callchain based on the global symbol_conf.use_callchain flag:
> > 
> > tools/perf/util/hist.c:hist_entry__delete() {
> >     ...
> >     if (symbol_conf.use_callchain)
> >         free_callchain(he->callchain);
> >     ...
> > }
> > 
> > Could this read past the end of the c2c_hist_entry structure and dereference
> > garbage memory in free_callchain_node()?
> 
> It looks like a real issue.  Can you please fix?  I'm not sure if c2c
> uses the callchains.

Oh, I saw it's handled in the patch 8.

Thanks,
Namhyung