linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Andi Kleen <andi@firstfloor.org>
To: acme@kernel.org
Cc: jolsa@kernel.org, linux-kernel@vger.kernel.org,
	Andi Kleen <ak@linux.intel.com>
Subject: [PATCH 1/3] perf, evlist: Fix access of freed id arrays
Date: Mon, 23 Sep 2019 16:33:37 -0700	[thread overview]
Message-ID: <20190923233339.25326-1-andi@firstfloor.org> (raw)

From: Andi Kleen <ak@linux.intel.com>

I'm not fully sure if this is the correct fix, but without this
I get crashes on more complex perf stat metric usages. The problem
is that part of the state gets freed when a weak group fails,
but then is later still used. Just don't free the ids, we're
going to reuse them anyways on the weak group retry.

For example:

% perf stat -M IpB,IpCall,IpTB,IPC,Retiring_SMT,Frontend_Bound_SMT,Kernel_Utilization,CPU_Utilization --metric-only -a -I 1000 sleep 2

crashes and gives in valgrind:

=21527== Invalid write of size 8
==21527==    at 0x4EE582: hlist_add_head (list.h:644)
==21527==    by 0x4EFD3C: perf_evlist__id_hash (evlist.c:477)
==21527==    by 0x4EFD99: perf_evlist__id_add (evlist.c:483)
==21527==    by 0x4EFF15: perf_evlist__id_add_fd (evlist.c:524)
==21527==    by 0x4FC693: store_evsel_ids (evsel.c:2969)
==21527==    by 0x4FC76C: perf_evsel__store_ids (evsel.c:2986)
==21527==    by 0x450DA7: __run_perf_stat (builtin-stat.c:519)
==21527==    by 0x451285: run_perf_stat (builtin-stat.c:636)
==21527==    by 0x454619: cmd_stat (builtin-stat.c:1966)
==21527==    by 0x4D557D: run_builtin (perf.c:310)
==21527==    by 0x4D57EA: handle_internal_command (perf.c:362)
==21527==    by 0x4D5931: run_argv (perf.c:406)
==21527==  Address 0x12e3f008 is 104 bytes inside a block of size 2,056 free'd
==21527==    at 0x4839A0C: free (vg_replace_malloc.c:540)
==21527==    by 0x627139: xyarray__delete (xyarray.c:32)
==21527==    by 0x4F6BE4: perf_evsel__free_id (evsel.c:1253)
==21527==    by 0x4FA11F: evsel__close (evsel.c:1994)
==21527==    by 0x4F30A3: perf_evlist__reset_weak_group (evlist.c:1783)
==21527==    by 0x450B47: __run_perf_stat (builtin-stat.c:466)
==21527==    by 0x451285: run_perf_stat (builtin-stat.c:636)
==21527==    by 0x454619: cmd_stat (builtin-stat.c:1966)
==21527==    by 0x4D557D: run_builtin (perf.c:310)
==21527==    by 0x4D57EA: handle_internal_command (perf.c:362)
==21527==    by 0x4D5931: run_argv (perf.c:406)
==21527==    by 0x4D5CAE: main (perf.c:531)
==21527==  Block was alloc'd at
==21527==    at 0x483AB1A: calloc (vg_replace_malloc.c:762)
==21527==    by 0x627024: zalloc (zalloc.c:8)
==21527==    by 0x627088: xyarray__new (xyarray.c:10)
==21527==    by 0x4F6B20: perf_evsel__alloc_id (evsel.c:1237)
==21527==    by 0x4FC74E: perf_evsel__store_ids (evsel.c:2983)
==21527==    by 0x450DA7: __run_perf_stat (builtin-stat.c:519)
==21527==    by 0x451285: run_perf_stat (builtin-stat.c:636)
==21527==    by 0x454619: cmd_stat (builtin-stat.c:1966)
==21527==    by 0x4D557D: run_builtin (perf.c:310)
==21527==    by 0x4D57EA: handle_internal_command (perf.c:362)
==21527==    by 0x4D5931: run_argv (perf.c:406)
==21527==    by 0x4D5CAE: main (perf.c:531)

Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 tools/perf/util/evlist.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 095924aa186b..765303553041 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -1780,7 +1780,7 @@ struct evsel *perf_evlist__reset_weak_group(struct evlist *evsel_list,
 			is_open = false;
 		if (c2->leader == leader) {
 			if (is_open)
-				evsel__close(c2);
+				perf_evsel__close(&evsel->core);
 			c2->leader = c2;
 			c2->core.nr_members = 0;
 		}
-- 
2.21.0


             reply	other threads:[~2019-09-23 23:33 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-23 23:33 Andi Kleen [this message]
2019-09-23 23:33 ` [PATCH 2/3] perf, expr: Remove assert usage Andi Kleen
2019-09-24  7:44   ` Jiri Olsa
2019-09-23 23:33 ` [PATCH 3/3] perf, stat: Fix free memory access / memory leaks in metrics Andi Kleen
2019-09-24  7:50   ` Jiri Olsa
2019-09-24 14:08     ` Andi Kleen
2019-09-24 14:44       ` Jiri Olsa
2019-09-24 19:20         ` Arnaldo Carvalho de Melo
2019-09-24  7:44 ` [PATCH 1/3] perf, evlist: Fix access of freed id arrays Jiri Olsa
2019-09-24 16:05   ` Andi Kleen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190923233339.25326-1-andi@firstfloor.org \
    --to=andi@firstfloor.org \
    --cc=acme@kernel.org \
    --cc=ak@linux.intel.com \
    --cc=jolsa@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).