lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Tue, 18 Jun 2019 14:39:37 +0800 From: Leo Yan <leo.yan@...aro.org> To: Arnaldo Carvalho de Melo <arnaldo.melo@...il.com> Cc: Alexander Shishkin <alexander.shishkin@...ux.intel.com>, Jiri Olsa <jolsa@...hat.com>, Namhyung Kim <namhyung@...nel.org>, Alexei Starovoitov <ast@...nel.org>, Daniel Borkmann <daniel@...earbox.net>, Martin KaFai Lau <kafai@...com>, Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>, linux-kernel@...r.kernel.org, netdev@...r.kernel.org, bpf@...r.kernel.org Subject: Re: [PATCH 2/2] perf trace: Handle NULL pointer dereference in trace__syscall_info() 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 <leo.yan@...aro.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 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 <leo.yan@...aro.org> 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)
Powered by blists - more mailing lists