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

[sashiko-bot@kernel.org]

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