Re: [PATCH v2 02/19] perf, tools: Fix buffer overflow while freeing events

Jiri Olsa
2017-08-22T08:20:28Z

On Fri, Aug 11, 2017 at 04:26:17PM -0700, Andi Kleen wrote:
> From: Andi Kleen <ak@linux.intel.com>
>
> 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 <jolsa@kernel.org>

thanks,
jirka