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: 11+ 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
2026-07-04 16:11 ` Namhyung Kim
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 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.