lkml.org 
[lkml]   [2019]   [Nov]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v2] perf tests: Fix out of bounds memory access
On Thu, Nov 07, 2019 at 10:02:44AM +0800, Leo Yan wrote:
> The test case 'Read backward ring buffer' failed on 32-bit architectures
> which were found by LKFT perf testing. The test failed on arm32 x15
> device, qemu_arm32, qemu_i386, and found intermittent failure on i386;
> the failure log is as below:
>
> 50: Read backward ring buffer :
> --- start ---
> test child forked, pid 510
> Using CPUID GenuineIntel-6-9E-9
> mmap size 1052672B
> mmap size 8192B
> Finished reading overwrite ring buffer: rewind
> free(): invalid next size (fast)
> test child interrupted
> ---- end ----
> Read backward ring buffer: FAILED!
>
> The log hints there have issue for memory usage, thus free() reports
> error 'invalid next size' and directly exit for the case. Finally, this
> issue is root caused as out of bounds memory access for the data array
> 'evsel->id'.
>
> The backward ring buffer test invokes do_test() twice. 'evsel->id' is
> allocated at the first call with the flow:
>
> test__backward_ring_buffer()
> `-> do_test()
> `-> evlist__mmap()
> `-> evlist__mmap_ex()
> `-> perf_evsel__alloc_id()
>
> So 'evsel->id' is allocated with one item, and it will be used in
> function perf_evlist__id_add():
>
> evsel->id[0] = id
> evsel->ids = 1
>
> At the second call for do_test(), it skips to initialize 'evsel->id'
> and reuses the array which is allocated in the first call. But
> 'evsel->ids' contains the stale value. Thus:
>
> evsel->id[1] = id -> out of bound access
> evsel->ids = 2
>
> To fix this issue, we will use evlist__open() and evlist__close() pair
> functions to prepare and cleanup context for evlist; so 'evsel->id' and
> 'evsel->ids' can be initialized properly when invoke do_test() and avoid
> the out of bounds memory access.

right, we need to solve this on libperf level, so it's possible
to call mmap/munmap multiple time without close/open.. I'll try
to send something, but meanwhile this is good workaround

Reviewed-by: Jiri Olsa <jolsa@kernel.org>

thanks,
jirka

>
> Signed-off-by: Leo Yan <leo.yan@linaro.org>
> ---
> tools/perf/tests/backward-ring-buffer.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/tools/perf/tests/backward-ring-buffer.c b/tools/perf/tests/backward-ring-buffer.c
> index 338cd9faa835..5128f727c0ef 100644
> --- a/tools/perf/tests/backward-ring-buffer.c
> +++ b/tools/perf/tests/backward-ring-buffer.c
> @@ -147,6 +147,15 @@ int test__backward_ring_buffer(struct test *test __maybe_unused, int subtest __m
> goto out_delete_evlist;
> }
>
> + evlist__close(evlist);
> +
> + err = evlist__open(evlist);
> + if (err < 0) {
> + pr_debug("perf_evlist__open: %s\n",
> + str_error_r(errno, sbuf, sizeof(sbuf)));
> + goto out_delete_evlist;
> + }
> +
> err = do_test(evlist, 1, &sample_count, &comm_count);
> if (err != TEST_OK)
> goto out_delete_evlist;
> --
> 2.17.1
>

\
 
 \ /
  Last update: 2019-11-07 10:43    [W:0.138 / U:25.972 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site