[PATCH] perf config: Use va_copy() in perf_config_scan_cb() to avoid va_list reuse

[PATCH] perf config: Use va_copy() in perf_config_scan_cb() to avoid va_list reuse

[Adrián García Casado]

perf_config() iterates through both the system-wide config file
(/etc/perfconfig) and the user config file (~/.perfconfig) so that user
settings override system defaults.

If a configuration key queried via perf_config_scan() appears more than
once (for example, in both /etc/perfconfig and ~/.perfconfig, or
duplicated within the same file), perf_config_scan_cb() invokes
vsscanf(value, d->fmt, d->args) multiple times on the same va_list
without reinitializing it.

Because vsscanf() consumes arguments from 'd->args' via va_arg(), a
subsequent match reads past the end of the argument list into
uninitialized stack memory.

Copy 'd->args' with va_copy() before calling vsscanf() in
perf_config_scan_cb() so that multiple matches safely overwrite the
destination variables with the latest value.

Signed-off-by: Adrián García Casado <adriangarciacasado42@gmail.com>
---
 tools/perf/util/config.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c
index 287401f159e66..f567cc338ca9d 100644
--- a/tools/perf/util/config.c
+++ b/tools/perf/util/config.c
@@ -1091,8 +1091,13 @@ static int perf_config_scan_cb(const char *var, const char *value, void *data)
 {
 	struct perf_config_scan_data *d = data;
 
-	if (!strcmp(var, d->name))
-		d->ret = vsscanf(value, d->fmt, d->args);
+	if (!strcmp(var, d->name)) {
+		va_list args;
+
+		va_copy(args, d->args);
+		d->ret = vsscanf(value, d->fmt, args);
+		va_end(args);
+	}
 
 	return 0;
 }
-- 
2.39.5

Re: [PATCH] perf config: Use va_copy() in perf_config_scan_cb() to avoid va_list reuse

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [Low] perf config: collect_config() already deduplicates keys for
  perf_config_scan()
- [Low] perf config: missing explicit stdarg.h include in
  tools/perf/util/config.c
--

commit f3806a1321b205e48a7070a279a393d0ecb60054
Author: Adrián García Casado <adriangarciacasado42@gmail.com>

perf config: Use va_copy() in perf_config_scan_cb() to avoid va_list reuse

This commit copies d->args with va_copy() before calling vsscanf() in
perf_config_scan_cb() so that repeated matches on a configuration key do not
reuse a consumed va_list.

> If a configuration key queried via perf_config_scan() appears more than
> once (for example, in both /etc/perfconfig and ~/.perfconfig, or
> duplicated within the same file), perf_config_scan_cb() invokes
> vsscanf(value, d->fmt, d->args) multiple times on the same va_list
> without reinitializing it.

[Severity: Low]
Can perf_config_scan_cb() actually be invoked more than once for the same
configuration key?

perf_config() builds a config_set via collect_config(), which looks up
existing entries with find_section() and find_config_item() and overwrites
item->value in place via set_value() when a duplicate key is parsed.
perf_config_set() then iterates over the deduplicated sections and items, so
perf_config_scan_cb() matches d->name at most once per perf_config_scan()
call.

Also, perf_config_get() already exists in tools/perf/util/config.c to look up
the resolved string for a key; calling perf_config_get() from
perf_config_scan() and running vsscanf() once on a non-NULL result would
avoid perf_config_scan_cb() and storing a va_list in struct
perf_config_scan_data altogether.

> diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c
> index 287401f159e66..f567cc338ca9d 100644
> --- a/tools/perf/util/config.c
> +++ b/tools/perf/util/config.c
> @@ -1091,8 +1091,13 @@ static int perf_config_scan_cb(const char *var, const char *value, void *data)
>  {
>  	struct perf_config_scan_data *d = data;
>  
> -	if (!strcmp(var, d->name))
> -		d->ret = vsscanf(value, d->fmt, d->args);
> +	if (!strcmp(var, d->name)) {
> +		va_list args;
> +
> +		va_copy(args, d->args);
> +		d->ret = vsscanf(value, d->fmt, args);
> +		va_end(args);
> +	}
>  
>  	return 0;
>  }

[Severity: Low]
This isn't a bug, but tools/perf/util/config.c uses va_list, va_start(),
va_copy(), and va_end() without explicitly including <stdarg.h> at the top of
the file, relying on transitive header inclusion that can break across libc
headers.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261109144210.1626450-1-adriangarciacasado42@gmail.com?part=1