From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932240AbdHVIU2 (ORCPT ); Tue, 22 Aug 2017 04:20:28 -0400 Received: from mx1.redhat.com ([209.132.183.28]:45038 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932077AbdHVIUZ (ORCPT ); Tue, 22 Aug 2017 04:20:25 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 5802A80F7A Authentication-Results: ext-mx03.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx03.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=jolsa@redhat.com Date: Tue, 22 Aug 2017 10:20:22 +0200 From: Jiri Olsa To: Andi Kleen Cc: acme@kernel.org, jolsa@kernel.org, linux-kernel@vger.kernel.org, Andi Kleen Subject: Re: [PATCH v2 02/19] perf, tools: Fix buffer overflow while freeing events Message-ID: <20170822082022.GB23985@krava> References: <20170811232634.30465-1-andi@firstfloor.org> <20170811232634.30465-2-andi@firstfloor.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170811232634.30465-2-andi@firstfloor.org> User-Agent: Mutt/1.8.3 (2017-05-23) X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.27]); Tue, 22 Aug 2017 08:20:25 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Aug 11, 2017 at 04:26:17PM -0700, Andi Kleen wrote: > From: Andi Kleen > > Fix buffer overflow for > > % perf stat -e msr/tsc/,cstate_core/c7-residency/ true > > that causes glibc free list corruption. For some reason > it doesn't trigger in valgrind, but it is visible in AS: > > ================================================================= > ==32681==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x603000003f5c at pc 0x0000005671ef bp 0x7ffdaaac9ac0 sp 0x7ffdaaac9ab0 > READ of size 4 at 0x603000003f5c thread T0 > #0 0x5671ee in perf_evsel__close_fd util/evsel.c:1196 > #1 0x56c57a in perf_evsel__close util/evsel.c:1717 > #2 0x55ed5f in perf_evlist__close util/evlist.c:1631 > #3 0x4647e1 in __run_perf_stat /home/ak/hle/linux-hle-2.6/tools/perf/builtin-stat.c:749 > #4 0x4648e3 in run_perf_stat /home/ak/hle/linux-hle-2.6/tools/perf/builtin-stat.c:767 > #5 0x46e1bc in cmd_stat /home/ak/hle/linux-hle-2.6/tools/perf/builtin-stat.c:2785 > #6 0x52f83d in run_builtin /home/ak/hle/linux-hle-2.6/tools/perf/perf.c:296 > #7 0x52fd49 in handle_internal_command /home/ak/hle/linux-hle-2.6/tools/perf/perf.c:348 > #8 0x5300de in run_argv /home/ak/hle/linux-hle-2.6/tools/perf/perf.c:392 > #9 0x5308f3 in main /home/ak/hle/linux-hle-2.6/tools/perf/perf.c:530 > #10 0x7f0672d13400 in __libc_start_main (/lib64/libc.so.6+0x20400) > #11 0x428419 in _start (/home/ak/hle/obj-perf/perf+0x428419) > > 0x603000003f5c is located 0 bytes to the right of 28-byte region [0x603000003f40,0x603000003f5c) > allocated by thread T0 here: > #0 0x7f0675139020 in calloc (/lib64/libasan.so.3+0xc7020) > #1 0x648a2d in zalloc util/util.h:23 > #2 0x648a88 in xyarray__new util/xyarray.c:9 > #3 0x566419 in perf_evsel__alloc_fd util/evsel.c:1039 > #4 0x56b427 in perf_evsel__open util/evsel.c:1529 > #5 0x56c620 in perf_evsel__open_per_thread util/evsel.c:1730 > #6 0x461dea in create_perf_stat_counter /home/ak/hle/linux-hle-2.6/tools/perf/builtin-stat.c:263 > #7 0x4637d7 in __run_perf_stat /home/ak/hle/linux-hle-2.6/tools/perf/builtin-stat.c:600 > #8 0x4648e3 in run_perf_stat /home/ak/hle/linux-hle-2.6/tools/perf/builtin-stat.c:767 > #9 0x46e1bc in cmd_stat /home/ak/hle/linux-hle-2.6/tools/perf/builtin-stat.c:2785 > #10 0x52f83d in run_builtin /home/ak/hle/linux-hle-2.6/tools/perf/perf.c:296 > #11 0x52fd49 in handle_internal_command /home/ak/hle/linux-hle-2.6/tools/perf/perf.c:348 > #12 0x5300de in run_argv /home/ak/hle/linux-hle-2.6/tools/perf/perf.c:392 > #13 0x5308f3 in main /home/ak/hle/linux-hle-2.6/tools/perf/perf.c:530 > #14 0x7f0672d13400 in __libc_start_main (/lib64/libc.so.6+0x20400) > > The event is allocated with cpus == 1, but freed with cpus == real number > When the evsel close function walks the file descriptors it exceeds the > fd xyarray boundaries and reads random memory. > > v2: > Now that xyarrays save their original dimensions we can use these > to iterate the two dimensional fd arrays. Fix some users > (close, ioctl) in evsel.c to use these fields directly. This allows simplifying > the code and dropping quite a few function arguments. Adjust > all callers by removing the unneeded arguments. > > The actual perf event reading still uses the original values from > the evsel list. i was wondering how much code change this would be, but it turned out not that bad ;-) Acked-by: Jiri Olsa thanks, jirka