On Mon, Jun 17, 2019 at 02:32:03PM -0300, Arnaldo Carvalho de Melo wrote: > Em Mon, Jun 17, 2019 at 05:11:40PM +0800, Leo Yan escreveu: > > trace__init_bpf_map_syscall_args() invokes trace__syscall_info() to > > retrieve system calls information, it always passes NULL for 'evsel' > > argument; when id is an invalid value then the logging will try to > > output event name, this triggers NULL pointer dereference. > > > > This patch directly uses string "unknown" for event name when 'evsel' > > is NULL pointer. > > > > Signed-off-by: Leo Yan <[email protected]> > > --- > > 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 5cd74651db4c..49dfb2fd393b 100644 > > --- a/tools/perf/builtin-trace.c > > +++ b/tools/perf/builtin-trace.c > > @@ -1764,7 +1764,7 @@ static struct syscall *trace__syscall_info(struct > > trace *trace, > > static u64 n; > > > > pr_debug("Invalid syscall %d id, skipping (%s, %" PRIu64 ")\n", > > - id, perf_evsel__name(evsel), ++n); > > + id, evsel ? perf_evsel__name(evsel) : "unknown", ++n); > > return NULL; > > What do you think of this instead?
Yes, I agree the below change is right thing to do. FWIW: Reviewed-by: Leo Yan <[email protected]> BTW, my patch followed the code in [1], after apply below your change, could consider to simplify code in [1] for without checking 'evsel' is NULL pointer anymore. Thanks, Leo [1] https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git/tree/tools/perf/builtin-report.c?h=perf/core#n301 > diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c > index 68beef8f47ff..1d6af95b9207 100644 > --- a/tools/perf/util/evsel.c > +++ b/tools/perf/util/evsel.c > @@ -590,6 +590,9 @@ const char *perf_evsel__name(struct perf_evsel *evsel) > { > char bf[128]; > > + if (!evsel) > + goto out_unknown; > + > if (evsel->name) > return evsel->name; > > @@ -629,7 +632,10 @@ const char *perf_evsel__name(struct perf_evsel *evsel) > > evsel->name = strdup(bf); > > - return evsel->name ?: "unknown"; > + if (evsel->name) > + return evsel->name; > +out_unknown: > + return "unknown"; > } > > const char *perf_evsel__group_name(struct perf_evsel *evsel)
