2024-05-09 05:31:38

by Ian Rogers

Subject: [PATCH v1] perf lock: Avoid memory leaks from strdup

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/[email protected]/

Suggested-by: Namhyung Kim <[email protected]>
Signed-off-by: Ian Rogers <[email protected]>
---
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

2024-05-09 18:24:32

by Namhyung Kim

Subject: Re: [PATCH v1] perf lock: Avoid memory leaks from strdup

On Wed, May 8, 2024 at 10:31 PM Ian Rogers <[email protected]> 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/[email protected]/
>
> Suggested-by: Namhyung Kim <[email protected]>
> Signed-off-by: Ian Rogers <[email protected]>

Acked-by: Namhyung Kim <[email protected]>

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
>

2024-05-10 14:16:05

by Arnaldo Carvalho de Melo

Subject: Re: [PATCH v1] perf lock: Avoid memory leaks from strdup

On Thu, May 09, 2024 at 11:24:10AM -0700, Namhyung Kim wrote:
> On Wed, May 8, 2024 at 10:31 PM Ian Rogers <[email protected]> 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/[email protected]/
> >
> > Suggested-by: Namhyung Kim <[email protected]>
> > Signed-off-by: Ian Rogers <[email protected]>
>
> Acked-by: Namhyung Kim <[email protected]>

Thanks, applied to perf-tools-next,

- Arnaldo