From: Namhyung Kim <namhyung@kernel.org>
To: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Ian Rogers <irogers@google.com>, Jiri Olsa <jolsa@kernel.org>,
Adrian Hunter <adrian.hunter@intel.com>,
James Clark <james.clark@linaro.org>,
Peter Zijlstra <peterz@infradead.org>,
Ingo Molnar <mingo@kernel.org>,
LKML <linux-kernel@vger.kernel.org>,
linux-perf-users@vger.kernel.org
Subject: [PATCH v3 3/6] perf kvm: Kill STRDUP_FAIL_EXIT()
Date: Wed, 1 Jul 2026 12:41:50 -0700 [thread overview]
Message-ID: <20260701194153.401218-4-namhyung@kernel.org> (raw)
In-Reply-To: <20260701194153.401218-1-namhyung@kernel.org>
It's used to pass command line options to a copied argv. But there's no
reason to make the copies as it's all used in the same function. It can
simply use stack variables.
In fact, it fixes a subtle double free issue. As parse_options() can
move contents in argv[], some entries may point to the same item. So
freeing all items in the argv could trigger a double free. With stack
variables, we don't need to allocate and free them.
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/builtin-kvm.c | 54 +++++++------------
.../util/kvm-stat-arch/kvm-stat-powerpc.c | 4 +-
tools/perf/util/kvm-stat-arch/kvm-stat-x86.c | 9 ++--
tools/perf/util/kvm-stat.h | 10 ----
4 files changed, 24 insertions(+), 53 deletions(-)
diff --git a/tools/perf/builtin-kvm.c b/tools/perf/builtin-kvm.c
index 24576e4ebb02cade..eaf9297cc34d32c1 100644
--- a/tools/perf/builtin-kvm.c
+++ b/tools/perf/builtin-kvm.c
@@ -1682,18 +1682,18 @@ kvm_events_record(struct perf_kvm_stat *kvm, int argc, const char **argv)
return -ENOMEM;
for (i = 0; i < ARRAY_SIZE(record_args); i++)
- rec_argv[i] = STRDUP_FAIL_EXIT(record_args[i]);
+ rec_argv[i] = record_args[i];
for (j = 0; j < events_tp_size; j++) {
- rec_argv[i++] = STRDUP_FAIL_EXIT("-e");
- rec_argv[i++] = STRDUP_FAIL_EXIT(kvm_events_tp(e_machine)[j]);
+ rec_argv[i++] = "-e";
+ rec_argv[i++] = kvm_events_tp(e_machine)[j];
}
- rec_argv[i++] = STRDUP_FAIL_EXIT("-o");
- rec_argv[i++] = STRDUP_FAIL_EXIT(kvm->file_name);
+ rec_argv[i++] = "-o";
+ rec_argv[i++] = kvm->file_name;
for (j = 1; j < (unsigned int)argc; j++, i++)
- rec_argv[i] = STRDUP_FAIL_EXIT(argv[j]);
+ rec_argv[i] = argv[j];
set_option_flag(record_options, 'e', "event", PARSE_OPT_HIDDEN);
set_option_flag(record_options, 0, "filter", PARSE_OPT_HIDDEN);
@@ -1717,10 +1717,6 @@ kvm_events_record(struct perf_kvm_stat *kvm, int argc, const char **argv)
record_usage = kvm_stat_record_usage;
ret = cmd_record(i, rec_argv);
-
-EXIT:
- for (i = 0; i < rec_argc; i++)
- free((void *)rec_argv[i]);
free(rec_argv);
return ret;
}
@@ -2008,9 +2004,9 @@ static int __cmd_record(const char *file_name, int argc, const char **argv)
if (!rec_argv)
return -ENOMEM;
- rec_argv[i++] = STRDUP_FAIL_EXIT("record");
- rec_argv[i++] = STRDUP_FAIL_EXIT("-o");
- rec_argv[i++] = STRDUP_FAIL_EXIT(file_name);
+ rec_argv[i++] = "record";
+ rec_argv[i++] = "-o";
+ rec_argv[i++] = file_name;
if (need_arch_event) {
ret = kvm_add_default_arch_event(EM_HOST, &i, rec_argv);
if (ret)
@@ -2018,15 +2014,13 @@ static int __cmd_record(const char *file_name, int argc, const char **argv)
}
for (j = 1; j < argc; j++, i++)
- rec_argv[i] = STRDUP_FAIL_EXIT(argv[j]);
+ rec_argv[i] = argv[j];
BUG_ON(i != rec_argc);
ret = cmd_record(i, rec_argv);
EXIT:
- for (j = 0; j < i; j++)
- free((void *)rec_argv[j]);
free(rec_argv);
return ret;
}
@@ -2041,19 +2035,15 @@ static int __cmd_report(const char *file_name, int argc, const char **argv)
if (!rec_argv)
return -ENOMEM;
- rec_argv[i++] = STRDUP_FAIL_EXIT("report");
- rec_argv[i++] = STRDUP_FAIL_EXIT("-i");
- rec_argv[i++] = STRDUP_FAIL_EXIT(file_name);
+ rec_argv[i++] = "report";
+ rec_argv[i++] = "-i";
+ rec_argv[i++] = file_name;
for (j = 1; j < argc; j++, i++)
- rec_argv[i] = STRDUP_FAIL_EXIT(argv[j]);
+ rec_argv[i] = argv[j];
BUG_ON(i != rec_argc);
ret = cmd_report(i, rec_argv);
-
-EXIT:
- for (i = 0; i < rec_argc; i++)
- free((void *)rec_argv[i]);
free(rec_argv);
return ret;
}
@@ -2069,19 +2059,15 @@ __cmd_buildid_list(const char *file_name, int argc, const char **argv)
if (!rec_argv)
return -ENOMEM;
- rec_argv[i++] = STRDUP_FAIL_EXIT("buildid-list");
- rec_argv[i++] = STRDUP_FAIL_EXIT("-i");
- rec_argv[i++] = STRDUP_FAIL_EXIT(file_name);
+ rec_argv[i++] = "buildid-list";
+ rec_argv[i++] = "-i";
+ rec_argv[i++] = file_name;
for (j = 1; j < argc; j++, i++)
- rec_argv[i] = STRDUP_FAIL_EXIT(argv[j]);
+ rec_argv[i] = argv[j];
BUG_ON(i != rec_argc);
ret = cmd_buildid_list(i, rec_argv);
-
-EXIT:
- for (i = 0; i < rec_argc; i++)
- free((void *)rec_argv[i]);
free(rec_argv);
return ret;
}
@@ -2101,7 +2087,7 @@ static int __cmd_top(int argc, const char **argv)
return -ENOMEM;
for (i = 0; i < argc; i++)
- rec_argv[i] = STRDUP_FAIL_EXIT(argv[i]);
+ rec_argv[i] = argv[i];
BUG_ON(i != argc);
@@ -2114,8 +2100,6 @@ static int __cmd_top(int argc, const char **argv)
ret = cmd_top(i, rec_argv);
EXIT:
- for (i = 0; i < rec_argc; i++)
- free((void *)rec_argv[i]);
free(rec_argv);
return ret;
}
diff --git a/tools/perf/util/kvm-stat-arch/kvm-stat-powerpc.c b/tools/perf/util/kvm-stat-arch/kvm-stat-powerpc.c
index 8d4133c35c12f14b..315b24f9a59297dc 100644
--- a/tools/perf/util/kvm-stat-arch/kvm-stat-powerpc.c
+++ b/tools/perf/util/kvm-stat-arch/kvm-stat-powerpc.c
@@ -181,8 +181,8 @@ int __kvm_add_default_arch_event_powerpc(int *argc, const char **argv)
if (!perf_pmus__have_event("trace_imc", "trace_cycles"))
return -EINVAL;
- argv[j++] = strdup("-e");
- argv[j++] = strdup("trace_imc/trace_cycles/");
+ argv[j++] = "-e";
+ argv[j++] = "trace_imc/trace_cycles/";
*argc += 2;
return 0;
diff --git a/tools/perf/util/kvm-stat-arch/kvm-stat-x86.c b/tools/perf/util/kvm-stat-arch/kvm-stat-x86.c
index 46f9a0adcb608aab..97041f5100fc812f 100644
--- a/tools/perf/util/kvm-stat-arch/kvm-stat-x86.c
+++ b/tools/perf/util/kvm-stat-arch/kvm-stat-x86.c
@@ -210,16 +210,13 @@ int __cpu_isa_init_x86(struct perf_kvm_stat *kvm, const char *cpuid)
*/
int __kvm_add_default_arch_event_x86(int *argc, const char **argv)
{
- int ret = 0, j = *argc;
+ int j = *argc;
- argv[j++] = STRDUP_FAIL_EXIT("-e");
- argv[j++] = STRDUP_FAIL_EXIT("cycles");
+ argv[j++] = "-e";
+ argv[j++] = "cycles";
*argc += 2;
return 0;
-
-EXIT:
- return ret;
}
const char * const *__kvm_events_tp_x86(void)
diff --git a/tools/perf/util/kvm-stat.h b/tools/perf/util/kvm-stat.h
index 04f9f3193555972f..1db31c0751875875 100644
--- a/tools/perf/util/kvm-stat.h
+++ b/tools/perf/util/kvm-stat.h
@@ -229,14 +229,4 @@ static inline struct kvm_info *kvm_info__new(void)
return ki;
}
-#define STRDUP_FAIL_EXIT(s) \
- ({ char *_p; \
- _p = strdup(s); \
- if (!_p) { \
- ret = -ENOMEM; \
- goto EXIT; \
- } \
- _p; \
- })
-
#endif /* __PERF_KVM_STAT_H */
--
2.55.0.rc0.799.gd6f94ed593-goog
next prev parent reply other threads:[~2026-07-01 19:41 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-01 19:41 [PATCH v3 0/6] perf kvm: A small update in the default arch event Namhyung Kim
2026-07-01 19:41 ` [PATCH v3 1/6] perf kvm: Factor out kvm_need_default_arch_event() Namhyung Kim
2026-07-01 19:59 ` sashiko-bot
2026-07-01 19:41 ` [PATCH v3 2/6] perf kvm: Check kvm_need_default_arch_event() early Namhyung Kim
2026-07-01 19:41 ` Namhyung Kim [this message]
2026-07-01 19:41 ` [PATCH v3 4/6] perf kvm: Do not copy filename string Namhyung Kim
2026-07-01 19:41 ` [PATCH v3 5/6] perf kvm: Fix a memory leak in the usage string Namhyung Kim
2026-07-01 19:41 ` [PATCH v3 6/6] perf test: Extend perf kvm tests to check default event Namhyung Kim
2026-07-01 20:11 ` sashiko-bot
2026-07-02 4:09 ` [PATCH v3 0/6] perf kvm: A small update in the default arch event Ian Rogers
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260701194153.401218-4-namhyung@kernel.org \
--to=namhyung@kernel.org \
--cc=acme@kernel.org \
--cc=adrian.hunter@intel.com \
--cc=irogers@google.com \
--cc=james.clark@linaro.org \
--cc=jolsa@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-perf-users@vger.kernel.org \
--cc=mingo@kernel.org \
--cc=peterz@infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox