diff mbox series

[v1,04/11] perf annotate: Smatch: Fix dereferencing freed memory

Message ID 20190702103420.27540-5-leo.yan@linaro.org (mailing list archive)
State New, archived
Headers show
Series perf: Fix errors detected by Smatch | expand

Commit Message

Leo Yan July 2, 2019, 10:34 a.m. UTC
Based on the following report from Smatch, fix the potential
dereferencing freed memory check.

  tools/perf/util/annotate.c:1125
  disasm_line__parse() error: dereferencing freed memory 'namep'

tools/perf/util/annotate.c
1100 static int disasm_line__parse(char *line, const char **namep, char **rawp)
1101 {
1102         char tmp, *name = ltrim(line);

[...]

1114         *namep = strdup(name);
1115
1116         if (*namep == NULL)
1117                 goto out_free_name;

[...]

1124 out_free_name:
1125         free((void *)namep);
                          ^^^^^
1126         *namep = NULL;
             ^^^^^^
1127         return -1;
1128 }

If strdup() fails to allocate memory space for *namep, we don't need to
free memory with pointer 'namep', which is resident in data structure
disasm_line::ins::name; and *namep is NULL pointer for this failure, so
it's pointless to assign NULL to *namep again.

Signed-off-by: Leo Yan <leo.yan@linaro.org>
---
 tools/perf/util/annotate.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

Comments

Arnaldo Carvalho de Melo July 3, 2019, 6:43 p.m. UTC | #1
Em Tue, Jul 02, 2019 at 06:34:13PM +0800, Leo Yan escreveu:
> Based on the following report from Smatch, fix the potential
> dereferencing freed memory check.
> 
>   tools/perf/util/annotate.c:1125
>   disasm_line__parse() error: dereferencing freed memory 'namep'
> 
> tools/perf/util/annotate.c
> 1100 static int disasm_line__parse(char *line, const char **namep, char **rawp)
> 1101 {
> 1102         char tmp, *name = ltrim(line);
> 
> [...]
> 
> 1114         *namep = strdup(name);
> 1115
> 1116         if (*namep == NULL)
> 1117                 goto out_free_name;
> 
> [...]
> 
> 1124 out_free_name:
> 1125         free((void *)namep);
>                           ^^^^^
> 1126         *namep = NULL;
>              ^^^^^^
> 1127         return -1;
> 1128 }
> 
> If strdup() fails to allocate memory space for *namep, we don't need to
> free memory with pointer 'namep', which is resident in data structure
> disasm_line::ins::name; and *namep is NULL pointer for this failure, so
> it's pointless to assign NULL to *namep again.

Applied, with this extra comment:


Committer note:

Freeing namep, which is the address of the first entry of the 'struct
ins' that is the first member of struct disasm_line would in fact free
that disasm_line instance, if it was allocated via malloc/calloc, which,
later, would a dereference of freed memory.
diff mbox series

Patch

diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index c8ce13419d9b..b8dfcfe08bb1 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -1113,16 +1113,14 @@  static int disasm_line__parse(char *line, const char **namep, char **rawp)
 	*namep = strdup(name);
 
 	if (*namep == NULL)
-		goto out_free_name;
+		goto out;
 
 	(*rawp)[0] = tmp;
 	*rawp = ltrim(*rawp);
 
 	return 0;
 
-out_free_name:
-	free((void *)namep);
-	*namep = NULL;
+out:
 	return -1;
 }