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

[Jiri Olsa <jolsa@redhat.com>]

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