perf symbols: Fix ownership of string in dso__load_vmlinux()
authorJames Clark <james.clark@arm.com>
Tue, 7 May 2024 14:12:08 +0000 (15:12 +0100)
committerArnaldo Carvalho de Melo <acme@redhat.com>
Thu, 9 May 2024 21:48:46 +0000 (18:48 -0300)
The linked commit updated dso__load_vmlinux() to call
dso__set_long_name() before loading the symbols. Loading the symbols may
not succeed but dso__set_long_name() takes ownership of the string. The
two callers of this function free the string themselves on failure
cases, resulting in the following error:

  $ perf record -- ls
  $ perf report

  free(): double free detected in tcache 2

Fix it by always taking ownership of the string, even on failure. This
means the string is either freed at the very first early exit condition,
or later when the dso is deleted or the long name is replaced. Now no
special return value is needed to signify that the caller needs to
free the string.

Fixes: e59fea47f83e8a9a ("perf symbols: Fix DSO kernel load and symbol process to correctly map DSO to its long_name, type and adjust_symbols")
Reviewed-by: Ian Rogers <irogers@google.com>
Signed-off-by: James Clark <james.clark@arm.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: https://lore.kernel.org/r/20240507141210.195939-5-james.clark@arm.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
tools/perf/util/symbol.c

index f21fe5772bd870f91cc06025cb3187bc5ed2982e..9e5940b5bc591d85d5cff6cdcffbc7c19da6b194 100644 (file)
@@ -1978,6 +1978,10 @@ out:
        return ret;
 }
 
+/*
+ * Always takes ownership of vmlinux when vmlinux_allocated == true, even if
+ * it returns an error.
+ */
 int dso__load_vmlinux(struct dso *dso, struct map *map,
                      const char *vmlinux, bool vmlinux_allocated)
 {
@@ -1996,8 +2000,11 @@ int dso__load_vmlinux(struct dso *dso, struct map *map,
        else
                symtab_type = DSO_BINARY_TYPE__VMLINUX;
 
-       if (symsrc__init(&ss, dso, symfs_vmlinux, symtab_type))
+       if (symsrc__init(&ss, dso, symfs_vmlinux, symtab_type)) {
+               if (vmlinux_allocated)
+                       free((char *) vmlinux);
                return -1;
+       }
 
        /*
         * dso__load_sym() may copy 'dso' which will result in the copies having
@@ -2040,7 +2047,6 @@ int dso__load_vmlinux_path(struct dso *dso, struct map *map)
                err = dso__load_vmlinux(dso, map, filename, true);
                if (err > 0)
                        goto out;
-               free(filename);
        }
 out:
        return err;
@@ -2192,7 +2198,6 @@ static int dso__load_kernel_sym(struct dso *dso, struct map *map)
                err = dso__load_vmlinux(dso, map, filename, true);
                if (err > 0)
                        return err;
-               free(filename);
        }
 
        if (!symbol_conf.ignore_vmlinux && vmlinux_path != NULL) {