Re: [PATCH v1 05/11] perf trace: Smatch: Fix potential NULL pointer dereference

Arnaldo Carvalho de Melo
2019-07-03T18:46:54Z

Em Tue, Jul 02, 2019 at 06:34:14PM +0800, Leo Yan escreveu:
> Based on the following report from Smatch, fix the potential
> NULL pointer dereference check.
>
> tools/perf/builtin-trace.c:1044
> thread_trace__new() error: we previously assumed 'ttrace' could be
> null (see line 1041).
>
> tools/perf/builtin-trace.c
> 1037 static struct thread_trace *thread_trace__new(void)
> 1038 {
> 1039 struct thread_trace *ttrace = zalloc(sizeof(struct thread_trace));
> 1040
> 1041 if (ttrace)
> 1042 ttrace->files.max = -1;
> 1043
> 1044 ttrace->syscall_stats = intlist__new(NULL);
> ^^^^^^^^
> 1045
> 1046 return ttrace;
> 1047 }
>
> This patch directly returns NULL when fail to allocate memory for
> ttrace; this can avoid potential NULL pointer dereference.

I did it slightly different, to avoid having two return lines, and that
is what most other constructors (foo__new()) do in tools/perf/, kept
your changeset comment and patch authorship, thanks.

diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
index d0eb7224dd36..e3fc9062f136 100644
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -1038,10 +1038,10 @@ static struct thread_trace *thread_trace__new(void)
{
struct thread_trace *ttrace = zalloc(sizeof(struct thread_trace));

- if (ttrace)
+ if (ttrace) {
ttrace->files.max = -1;
-
- ttrace->syscall_stats = intlist__new(NULL);
+ ttrace->syscall_stats = intlist__new(NULL);
+ }

return ttrace;
}