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

[Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com>]

Em Fri, Aug 09, 2019 at 09:44:31PM +0800, Leo Yan escreveu:
> On Fri, Aug 09, 2019 at 10:25:22AM -0300, Arnaldo Carvalho de Melo wrote:
> > 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!
> 
> You are welcome, Arnaldo.
> 
> > 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]$
> 
> Thanks!  Yes, I am working on perf/core branch and hit this issue.
> 
> Just in case Ingo has not merged your PR, if could save your efforts
> it's quite fine for me to merge this change in your original patch.

This got already merged into tip/perf/core, so no way to combine both by
now, unfortunately, would be good for bisection purposes on ARM64,
agreed, but not possible.

Thanks again,

- Arnaldo
 
> Thanks,
> Leo Yan
> 
> > 
> > - 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

-- 

- Arnaldo