perf kvm: Kill STRDUP_FAIL_EXIT()

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.

Tested-by: Ian Rogers <irogers@google.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
This commit is contained in:
Namhyung Kim
2026-07-03 13:40:24 -07:00
parent 95e6323838
commit cd589eba99
4 changed files with 24 additions and 53 deletions
+19 -35
View File
@@ -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;
}
@@ -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;
+3 -6
View File
@@ -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)
-10
View File
@@ -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 */