Re: [PATCH] perf trace: Fix segmentation fault when access syscall info

From: Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com>
To: Leo Yan <leo.yan@linaro.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Jiri Olsa <jolsa@redhat.com>, Namhyung Kim <namhyung@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Martin KaFai Lau <kafai@fb.com>, Song Liu <songliubraving@fb.com>,
	Yonghong Song <yhs@fb.com>,
	linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
	bpf@vger.kernel.org
Subject: Re: [PATCH] perf trace: Fix segmentation fault when access syscall info
Date: Fri, 9 Aug 2019 10:25:22 -0300	[thread overview]
Message-ID: <20190809132522.GB20899@kernel.org> (raw)
In-Reply-To: <20190809104752.27338-1-leo.yan@linaro.org>

Em Fri, Aug 09, 2019 at 06:47:52PM +0800, Leo Yan escreveu:
> 'perf trace' reports the segmentation fault as below on Arm64:
> 
>   # perf trace -e string -e augmented_raw_syscalls.c
>   LLVM: dumping tools/perf/examples/bpf/augmented_raw_syscalls.o
>   perf: Segmentation fault
>   Obtained 12 stack frames.
>   perf(sighandler_dump_stack+0x47) [0xaaaaac96ac87]
>   linux-vdso.so.1(+0x5b7) [0xffffadbeb5b7]
>   /lib/aarch64-linux-gnu/libc.so.6(strlen+0x10) [0xfffface7d5d0]
>   /lib/aarch64-linux-gnu/libc.so.6(_IO_vfprintf+0x1ac7) [0xfffface49f97]
>   /lib/aarch64-linux-gnu/libc.so.6(__vsnprintf_chk+0xc7) [0xffffacedfbe7]
>   perf(scnprintf+0x97) [0xaaaaac9ca3ff]
>   perf(+0x997bb) [0xaaaaac8e37bb]
>   perf(cmd_trace+0x28e7) [0xaaaaac8ec09f]
>   perf(+0xd4a13) [0xaaaaac91ea13]
>   perf(main+0x62f) [0xaaaaac8a147f]
>   /lib/aarch64-linux-gnu/libc.so.6(__libc_start_main+0xe3) [0xfffface22d23]
>   perf(+0x57723) [0xaaaaac8a1723]
>   Segmentation fault
> 
> This issue is introduced by commit 30a910d7d3e0 ("perf trace:
> Preallocate the syscall table"), it allocates trace->syscalls.table[]
> array and the element count is 'trace->sctbl->syscalls.nr_entries';
> but on Arm64, the system call number is not continuously used; e.g. the
> syscall maximum id is 436 but the real entries is only 281.  So the
> table is allocated with 'nr_entries' as the element count, but it
> accesses the table with the syscall id, which might be out of the bound
> of the array and cause the segmentation fault.
> 
> This patch allocates trace->syscalls.table[] with the element count is
> 'trace->sctbl->syscalls.max_id + 1', this allows any id to access the
> table without out of the bound.

Thanks a lot! My bad, that is why we have that max_id there, I forgot
about it and since I tested so far only on x86_64... applied to
perf/core, since it is only on:

[acme@quaco perf]$ git tag --contains 30a910d7d3e0
perf-core-for-mingo-5.4-20190729
[acme@quaco perf]$

- Arnaldo

> Fixes: 30a910d7d3e0 ("perf trace: Preallocate the syscall table")
> Signed-off-by: Leo Yan <leo.yan@linaro.org>
> ---
>  tools/perf/builtin-trace.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
> index 75eb3811e942..d553d06a9aeb 100644
> --- a/tools/perf/builtin-trace.c
> +++ b/tools/perf/builtin-trace.c
> @@ -1492,7 +1492,7 @@ static int trace__read_syscall_info(struct trace *trace, int id)
>  	const char *name = syscalltbl__name(trace->sctbl, id);
>  
>  	if (trace->syscalls.table == NULL) {
> -		trace->syscalls.table = calloc(trace->sctbl->syscalls.nr_entries, sizeof(*sc));
> +		trace->syscalls.table = calloc(trace->sctbl->syscalls.max_id + 1, sizeof(*sc));
>  		if (trace->syscalls.table == NULL)
>  			return -ENOMEM;
>  	}
> -- 
> 2.17.1

-- 

- Arnaldo