Received-SPF: pass (google.com: domain of linux-kernel+bounces-174766-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Precedence: bulk
MIME-Version: 1.0
References: <20240509053123.1918093-1-irogers@google.com>
In-Reply-To: <20240509053123.1918093-1-irogers@google.com>
From: Namhyung Kim <namhyung@kernel.org>
Date: Thu, 9 May 2024 11:24:10 -0700
Message-ID: <CAM9d7cgvRO0niWKi8dVnaYU1Y0C2hWCYxdA+Yb1vObDdgK85Kg@mail.gmail.com>
Subject: Re: [PATCH v1] perf lock: Avoid memory leaks from strdup
To: Ian Rogers <irogers@google.com>
Cc: zhaimingbing <zhaimingbing@cmss.chinamobile.com>, 
	Peter Zijlstra <peterz@infradead.org>, Ingo Molnar <mingo@redhat.com>, 
	Arnaldo Carvalho de Melo <acme@kernel.org>, Mark Rutland <mark.rutland@arm.com>, 
	Alexander Shishkin <alexander.shishkin@linux.intel.com>, Jiri Olsa <jolsa@kernel.org>, 
	Adrian Hunter <adrian.hunter@intel.com>, Kan Liang <kan.liang@linux.intel.com>, 
	linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Wed, May 8, 2024 at 10:31=E2=80=AFPM Ian Rogers <irogers@google.com> wro=
te:
>
> 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 **ar=
gv)
>                 return -ENOMEM;
>
>         for (i =3D 0; i < ARRAY_SIZE(record_args); i++)
> -               rec_argv[i] =3D strdup(record_args[i]);
> +               rec_argv[i] =3D record_args[i];
>
>         for (j =3D 0; j < nr_tracepoints; j++) {
> -               const char *ev_name;
> -
> -               if (has_lock_stat)
> -                       ev_name =3D strdup(lock_tracepoints[j].name);
> -               else
> -                       ev_name =3D strdup(contention_tracepoints[j].name=
);
> -
> -               if (!ev_name) {
> -                       free(rec_argv);
> -                       return -ENOMEM;
> -               }
> -
>                 rec_argv[i++] =3D "-e";
> -               rec_argv[i++] =3D ev_name;
> +               rec_argv[i++] =3D has_lock_stat
> +                       ? lock_tracepoints[j].name
> +                       : contention_tracepoints[j].name;
>         }
>
>         for (j =3D 0; j < nr_callgraph_args; j++, i++)
> --
> 2.45.0.rc1.225.g2a3ae87e7f-goog
>