All of lore.kernel.org
 help / color / mirror / Atom feed
* [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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.