* [PATCH] perf report: Fix memory leaks around perf_tip
@ 2021-11-18 7:38 Ian Rogers
2021-11-18 13:18 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 2+ messages in thread
From: Ian Rogers @ 2021-11-18 7:38 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
linux-perf-users, linux-kernel
Cc: Stephane Eranian, Ian Rogers
perf_tip may allocate memory or use a literal, this means memory wasn't
freed if allocated. Change the API so that literals aren't used. At the
same time add missing frees for system_path. These issues were spotted
using leak sanitizer.
Signed-off-by: Ian Rogers <irogers@google.com>
---
tools/perf/builtin-report.c | 15 +++++++++------
tools/perf/util/util.c | 14 +++++++-------
tools/perf/util/util.h | 2 +-
3 files changed, 17 insertions(+), 14 deletions(-)
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 8167ebfe776a..8ae400429870 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -619,14 +619,17 @@ static int report__browse_hists(struct report *rep)
int ret;
struct perf_session *session = rep->session;
struct evlist *evlist = session->evlist;
- const char *help = perf_tip(system_path(TIPDIR));
+ char *help = NULL, *path = NULL;
- if (help == NULL) {
+ path = system_path(TIPDIR);
+ if (perf_tip(&help, path) || help == NULL) {
/* fallback for people who don't install perf ;-) */
- help = perf_tip(DOCDIR);
- if (help == NULL)
- help = "Cannot load tips.txt file, please install perf!";
+ free(path);
+ path = system_path(DOCDIR);
+ if (perf_tip(&help, path) || help == NULL)
+ help = strdup("Cannot load tips.txt file, please install perf!");
}
+ free(path);
switch (use_browser) {
case 1:
@@ -651,7 +654,7 @@ static int report__browse_hists(struct report *rep)
ret = evlist__tty_browse_hists(evlist, rep, help);
break;
}
-
+ free(help);
return ret;
}
diff --git a/tools/perf/util/util.c b/tools/perf/util/util.c
index 37a9492edb3e..df3c4671be72 100644
--- a/tools/perf/util/util.c
+++ b/tools/perf/util/util.c
@@ -379,32 +379,32 @@ fetch_kernel_version(unsigned int *puint, char *str,
return 0;
}
-const char *perf_tip(const char *dirpath)
+int perf_tip(char **strp, const char *dirpath)
{
struct strlist *tips;
struct str_node *node;
- char *tip = NULL;
struct strlist_config conf = {
.dirname = dirpath,
.file_only = true,
};
+ int ret = 0;
+ *strp = NULL;
tips = strlist__new("tips.txt", &conf);
if (tips == NULL)
- return errno == ENOENT ? NULL :
- "Tip: check path of tips.txt or get more memory! ;-p";
+ return -errno;
if (strlist__nr_entries(tips) == 0)
goto out;
node = strlist__entry(tips, random() % strlist__nr_entries(tips));
- if (asprintf(&tip, "Tip: %s", node->s) < 0)
- tip = (char *)"Tip: get more memory! ;-)";
+ if (asprintf(strp, "Tip: %s", node->s) < 0)
+ ret = -ENOMEM;
out:
strlist__delete(tips);
- return tip;
+ return ret;
}
char *perf_exe(char *buf, int len)
diff --git a/tools/perf/util/util.h b/tools/perf/util/util.h
index ad737052e597..9f0d36ba77f2 100644
--- a/tools/perf/util/util.h
+++ b/tools/perf/util/util.h
@@ -39,7 +39,7 @@ int fetch_kernel_version(unsigned int *puint,
#define KVER_FMT "%d.%d.%d"
#define KVER_PARAM(x) KVER_VERSION(x), KVER_PATCHLEVEL(x), KVER_SUBLEVEL(x)
-const char *perf_tip(const char *dirpath);
+int perf_tip(char **strp, const char *dirpath);
#ifndef HAVE_SCHED_GETCPU_SUPPORT
int sched_getcpu(void);
--
2.34.0.rc1.387.gb447b232ab-goog
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] perf report: Fix memory leaks around perf_tip
2021-11-18 7:38 [PATCH] perf report: Fix memory leaks around perf_tip Ian Rogers
@ 2021-11-18 13:18 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 2+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-11-18 13:18 UTC (permalink / raw)
To: Ian Rogers
Cc: Peter Zijlstra, Ingo Molnar, Mark Rutland, Alexander Shishkin,
Jiri Olsa, Namhyung Kim, linux-perf-users, linux-kernel,
Stephane Eranian
Em Wed, Nov 17, 2021 at 11:38:04PM -0800, Ian Rogers escreveu:
> perf_tip may allocate memory or use a literal, this means memory wasn't
> freed if allocated. Change the API so that literals aren't used. At the
> same time add missing frees for system_path. These issues were spotted
> using leak sanitizer.
Thanks, applied.
- Arnaldo
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
> tools/perf/builtin-report.c | 15 +++++++++------
> tools/perf/util/util.c | 14 +++++++-------
> tools/perf/util/util.h | 2 +-
> 3 files changed, 17 insertions(+), 14 deletions(-)
>
> diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
> index 8167ebfe776a..8ae400429870 100644
> --- a/tools/perf/builtin-report.c
> +++ b/tools/perf/builtin-report.c
> @@ -619,14 +619,17 @@ static int report__browse_hists(struct report *rep)
> int ret;
> struct perf_session *session = rep->session;
> struct evlist *evlist = session->evlist;
> - const char *help = perf_tip(system_path(TIPDIR));
> + char *help = NULL, *path = NULL;
>
> - if (help == NULL) {
> + path = system_path(TIPDIR);
> + if (perf_tip(&help, path) || help == NULL) {
> /* fallback for people who don't install perf ;-) */
> - help = perf_tip(DOCDIR);
> - if (help == NULL)
> - help = "Cannot load tips.txt file, please install perf!";
> + free(path);
> + path = system_path(DOCDIR);
> + if (perf_tip(&help, path) || help == NULL)
> + help = strdup("Cannot load tips.txt file, please install perf!");
> }
> + free(path);
>
> switch (use_browser) {
> case 1:
> @@ -651,7 +654,7 @@ static int report__browse_hists(struct report *rep)
> ret = evlist__tty_browse_hists(evlist, rep, help);
> break;
> }
> -
> + free(help);
> return ret;
> }
>
> diff --git a/tools/perf/util/util.c b/tools/perf/util/util.c
> index 37a9492edb3e..df3c4671be72 100644
> --- a/tools/perf/util/util.c
> +++ b/tools/perf/util/util.c
> @@ -379,32 +379,32 @@ fetch_kernel_version(unsigned int *puint, char *str,
> return 0;
> }
>
> -const char *perf_tip(const char *dirpath)
> +int perf_tip(char **strp, const char *dirpath)
> {
> struct strlist *tips;
> struct str_node *node;
> - char *tip = NULL;
> struct strlist_config conf = {
> .dirname = dirpath,
> .file_only = true,
> };
> + int ret = 0;
>
> + *strp = NULL;
> tips = strlist__new("tips.txt", &conf);
> if (tips == NULL)
> - return errno == ENOENT ? NULL :
> - "Tip: check path of tips.txt or get more memory! ;-p";
> + return -errno;
>
> if (strlist__nr_entries(tips) == 0)
> goto out;
>
> node = strlist__entry(tips, random() % strlist__nr_entries(tips));
> - if (asprintf(&tip, "Tip: %s", node->s) < 0)
> - tip = (char *)"Tip: get more memory! ;-)";
> + if (asprintf(strp, "Tip: %s", node->s) < 0)
> + ret = -ENOMEM;
>
> out:
> strlist__delete(tips);
>
> - return tip;
> + return ret;
> }
>
> char *perf_exe(char *buf, int len)
> diff --git a/tools/perf/util/util.h b/tools/perf/util/util.h
> index ad737052e597..9f0d36ba77f2 100644
> --- a/tools/perf/util/util.h
> +++ b/tools/perf/util/util.h
> @@ -39,7 +39,7 @@ int fetch_kernel_version(unsigned int *puint,
> #define KVER_FMT "%d.%d.%d"
> #define KVER_PARAM(x) KVER_VERSION(x), KVER_PATCHLEVEL(x), KVER_SUBLEVEL(x)
>
> -const char *perf_tip(const char *dirpath);
> +int perf_tip(char **strp, const char *dirpath);
>
> #ifndef HAVE_SCHED_GETCPU_SUPPORT
> int sched_getcpu(void);
> --
> 2.34.0.rc1.387.gb447b232ab-goog
--
- Arnaldo
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2021-11-18 13:18 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-18 7:38 [PATCH] perf report: Fix memory leaks around perf_tip Ian Rogers
2021-11-18 13:18 ` Arnaldo Carvalho de Melo
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).