LKML Archive mirror
 help / color / mirror / Atom feed
* [PATCH v1] perf lock: Avoid memory leaks from strdup
@ 2024-05-09  5:31 Ian Rogers
  2024-05-09 18:24 ` Namhyung Kim
  0 siblings, 1 reply; 3+ messages in thread
From: Ian Rogers @ 2024-05-09  5:31 UTC (permalink / raw
  To: zhaimingbing, Peter Zijlstra, Ingo Molnar,
	Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
	Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
	Kan Liang, linux-perf-users, linux-kernel

Leak sanitizer complains about the strdup-ed arguments not being freed
and given cmd_record doesn't modify the given strings, remove the
strdups.

Original discussion in this patch:
https://lore.kernel.org/lkml/20240430184156.1824083-1-irogers@google.com/

Suggested-by: Namhyung Kim <namhyung@kernel.org>
Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/builtin-lock.c | 18 ++++--------------
 1 file changed, 4 insertions(+), 14 deletions(-)

diff --git a/tools/perf/builtin-lock.c b/tools/perf/builtin-lock.c
index 230461280e45..7007d26fe654 100644
--- a/tools/perf/builtin-lock.c
+++ b/tools/perf/builtin-lock.c
@@ -2275,23 +2275,13 @@ static int __cmd_record(int argc, const char **argv)
 		return -ENOMEM;
 
 	for (i = 0; i < ARRAY_SIZE(record_args); i++)
-		rec_argv[i] = strdup(record_args[i]);
+		rec_argv[i] = record_args[i];
 
 	for (j = 0; j < nr_tracepoints; j++) {
-		const char *ev_name;
-
-		if (has_lock_stat)
-			ev_name = strdup(lock_tracepoints[j].name);
-		else
-			ev_name = strdup(contention_tracepoints[j].name);
-
-		if (!ev_name) {
-			free(rec_argv);
-			return -ENOMEM;
-		}
-
 		rec_argv[i++] = "-e";
-		rec_argv[i++] = ev_name;
+		rec_argv[i++] = has_lock_stat
+			? lock_tracepoints[j].name
+			: contention_tracepoints[j].name;
 	}
 
 	for (j = 0; j < nr_callgraph_args; j++, i++)
-- 
2.45.0.rc1.225.g2a3ae87e7f-goog


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v1] perf lock: Avoid memory leaks from strdup
  2024-05-09  5:31 [PATCH v1] perf lock: Avoid memory leaks from strdup Ian Rogers
@ 2024-05-09 18:24 ` Namhyung Kim
  2024-05-10 14:15   ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 3+ messages in thread
From: Namhyung Kim @ 2024-05-09 18:24 UTC (permalink / raw
  To: Ian Rogers
  Cc: zhaimingbing, Peter Zijlstra, Ingo Molnar,
	Arnaldo Carvalho de Melo, Mark Rutland, Alexander Shishkin,
	Jiri Olsa, Adrian Hunter, Kan Liang, linux-perf-users,
	linux-kernel

On Wed, May 8, 2024 at 10:31 PM Ian Rogers <irogers@google.com> wrote:
>
> Leak sanitizer complains about the strdup-ed arguments not being freed
> and given cmd_record doesn't modify the given strings, remove the
> strdups.
>
> Original discussion in this patch:
> https://lore.kernel.org/lkml/20240430184156.1824083-1-irogers@google.com/
>
> Suggested-by: Namhyung Kim <namhyung@kernel.org>
> Signed-off-by: Ian Rogers <irogers@google.com>

Acked-by: Namhyung Kim <namhyung@kernel.org>

Thanks,
Namhyung


> ---
>  tools/perf/builtin-lock.c | 18 ++++--------------
>  1 file changed, 4 insertions(+), 14 deletions(-)
>
> diff --git a/tools/perf/builtin-lock.c b/tools/perf/builtin-lock.c
> index 230461280e45..7007d26fe654 100644
> --- a/tools/perf/builtin-lock.c
> +++ b/tools/perf/builtin-lock.c
> @@ -2275,23 +2275,13 @@ static int __cmd_record(int argc, const char **argv)
>                 return -ENOMEM;
>
>         for (i = 0; i < ARRAY_SIZE(record_args); i++)
> -               rec_argv[i] = strdup(record_args[i]);
> +               rec_argv[i] = record_args[i];
>
>         for (j = 0; j < nr_tracepoints; j++) {
> -               const char *ev_name;
> -
> -               if (has_lock_stat)
> -                       ev_name = strdup(lock_tracepoints[j].name);
> -               else
> -                       ev_name = strdup(contention_tracepoints[j].name);
> -
> -               if (!ev_name) {
> -                       free(rec_argv);
> -                       return -ENOMEM;
> -               }
> -
>                 rec_argv[i++] = "-e";
> -               rec_argv[i++] = ev_name;
> +               rec_argv[i++] = has_lock_stat
> +                       ? lock_tracepoints[j].name
> +                       : contention_tracepoints[j].name;
>         }
>
>         for (j = 0; j < nr_callgraph_args; j++, i++)
> --
> 2.45.0.rc1.225.g2a3ae87e7f-goog
>

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v1] perf lock: Avoid memory leaks from strdup
  2024-05-09 18:24 ` Namhyung Kim
@ 2024-05-10 14:15   ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 3+ messages in thread
From: Arnaldo Carvalho de Melo @ 2024-05-10 14:15 UTC (permalink / raw
  To: Namhyung Kim
  Cc: Ian Rogers, zhaimingbing, Peter Zijlstra, Ingo Molnar,
	Mark Rutland, Alexander Shishkin, Jiri Olsa, Adrian Hunter,
	Kan Liang, linux-perf-users, linux-kernel

On Thu, May 09, 2024 at 11:24:10AM -0700, Namhyung Kim wrote:
> On Wed, May 8, 2024 at 10:31 PM Ian Rogers <irogers@google.com> wrote:
> >
> > Leak sanitizer complains about the strdup-ed arguments not being freed
> > and given cmd_record doesn't modify the given strings, remove the
> > strdups.
> >
> > Original discussion in this patch:
> > https://lore.kernel.org/lkml/20240430184156.1824083-1-irogers@google.com/
> >
> > Suggested-by: Namhyung Kim <namhyung@kernel.org>
> > Signed-off-by: Ian Rogers <irogers@google.com>
> 
> Acked-by: Namhyung Kim <namhyung@kernel.org>

Thanks, applied to perf-tools-next,

- Arnaldo

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2024-05-10 14:15 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-05-09  5:31 [PATCH v1] perf lock: Avoid memory leaks from strdup Ian Rogers
2024-05-09 18:24 ` Namhyung Kim
2024-05-10 14:15   ` 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 read-only IMAP folder(s) and NNTP newsgroup(s).