linux-kernel - Re: [PATCH v1 06/17] perf hist: In init, ensure mem

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAP-5=fUayM2OY5rV==dpWSQkbPO4pOLs+RtzUC1W2LexnTNc_A@mail.gmail.com>
Date: Mon, 1 Dec 2025 09:52:49 -0800
From: Ian Rogers <irogers@...gle.com>
To: Arnaldo Carvalho de Melo <acme@...nel.org>
Cc: Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>, 
	Namhyung Kim <namhyung@...nel.org>, 
	Alexander Shishkin <alexander.shishkin@...ux.intel.com>, Jiri Olsa <jolsa@...nel.org>, 
	Adrian Hunter <adrian.hunter@...el.com>, James Clark <james.clark@...aro.org>, 
	Collin Funk <collin.funk1@...il.com>, Dmitry Vyukov <dvyukov@...gle.com>, 
	Andi Kleen <ak@...ux.intel.com>, Thomas Falcon <thomas.falcon@...el.com>, Leo Yan <leo.yan@....com>, 
	Yicong Yang <yangyicong@...ilicon.com>, Greg Kroah-Hartman <gregkh@...uxfoundation.org>, 
	"Masami Hiramatsu (Google)" <mhiramat@...nel.org>, Stephen Brennan <stephen.s.brennan@...cle.com>, 
	Haibo Xu <haibo1.xu@...el.com>, linux-kernel@...r.kernel.org, 
	linux-perf-users@...r.kernel.org
Subject: Re: [PATCH v1 06/17] perf hist: In init, ensure mem_info is put on
 error paths

On Mon, Dec 1, 2025 at 9:14 AM Arnaldo Carvalho de Melo <acme@...nel.org> wrote:
>
> On Sat, Nov 22, 2025 at 12:19:18AM -0800, Ian Rogers wrote:
> > Rather than exit the internal map_symbols directly, put the mem-info
> > that does this and also lowers the reference count on the mem-info
> > itself otherwise the mem-info is being leaked.
>
> Shouldn't this have a:
>
> Fixes: 56e144fe98260a0f ("perf mem_info: Add and use map_symbol__exit and addr_map_symbol__exit")

I don't mind the fixes tag. The change in that patch was:
```
        if (he->mem_info) {
-               map__put(he->mem_info->iaddr.ms.map);
-               map__put(he->mem_info->daddr.ms.map);
+               map_symbol__exit(&he->mem_info->iaddr.ms);
+               map_symbol__exit(&he->mem_info->daddr.ms);
       }
```
and so technically the memory leak predated that change.

Thanks,
Ian

> Other than that:
>
> Reviewed-by: Arnaldo Carvalho de Melo <acme@...hat.com>
>
> - Arnaldo
>
> > Signed-off-by: Ian Rogers <irogers@...gle.com>
> > ---
> >  tools/perf/util/hist.c | 6 ++----
> >  1 file changed, 2 insertions(+), 4 deletions(-)
> >
> > diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
> > index 64ff427040c3..ef4b569f7df4 100644
> > --- a/tools/perf/util/hist.c
> > +++ b/tools/perf/util/hist.c
> > @@ -608,10 +608,8 @@ static int hist_entry__init(struct hist_entry *he,
> >               map_symbol__exit(&he->branch_info->to.ms);
> >               zfree(&he->branch_info);
> >       }
> > -     if (he->mem_info) {
> > -             map_symbol__exit(&mem_info__iaddr(he->mem_info)->ms);
> > -             map_symbol__exit(&mem_info__daddr(he->mem_info)->ms);
> > -     }
> > +     if (he->mem_info)
> > +             mem_info__zput(he->mem_info);
> >  err:
> >       map_symbol__exit(&he->ms);
> >       zfree(&he->stat_acc);
> > --
> > 2.52.0.rc2.455.g230fcf2819-goog
> >