[PATCH 3/4] perf kvm: Kill STRDUP_FAIL_EXIT()
From: Namhyung Kim
Date: Tue Jun 23 2026 - 03:03:42 EST
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@xxxxxxxxxx>
---
tools/perf/builtin-kvm.c | 54 +++++++-------------
tools/perf/util/kvm-stat-arch/kvm-stat-x86.c | 9 ++--
tools/perf/util/kvm-stat.h | 10 ----
3 files changed, 22 insertions(+), 51 deletions(-)
diff --git a/tools/perf/builtin-kvm.c b/tools/perf/builtin-kvm.c
index cec26f898bf6eaeb..84f9ce4481f73842 100644
--- a/tools/perf/builtin-kvm.c
+++ b/tools/perf/builtin-kvm.c
@@ -1681,18 +1681,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);
@@ -1716,10 +1716,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;
}
@@ -2007,9 +2003,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)
@@ -2017,15 +2013,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;
}
@@ -2040,19 +2034,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;
}
@@ -2068,19 +2058,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;
}
@@ -2100,7 +2086,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);
@@ -2111,8 +2097,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-x86.c b/tools/perf/util/kvm-stat-arch/kvm-stat-x86.c
index 7bef7657a68a3e55..923b157bec2979b0 100644
--- a/tools/perf/util/kvm-stat-arch/kvm-stat-x86.c
+++ b/tools/perf/util/kvm-stat-arch/kvm-stat-x86.c
@@ -210,19 +210,16 @@ 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;
if (!x86__is_intel_cpu())
return 0;
- 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 111b02fd0878efc2..47bb0bfcb9bf6b53 100644
--- a/tools/perf/util/kvm-stat.h
+++ b/tools/perf/util/kvm-stat.h
@@ -228,14 +228,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.54.0