Re: [PATCH 2/4] perf kvm: Check kvm_need_default_arch_event() early

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] BUG_ON() unconditionally triggers and crashes `perf kvm record` on architectures where default arch events are not added (e.g., ARM64, RISC-V, s390).
--

commit e180ad0afcd0778bb15a60785a0b08bfd2f76a50
Author: Namhyung Kim <namhyung@kernel.org>

perf kvm: Check kvm_need_default_arch_event() early

This commit modifies perf kvm record to check if default options are needed
before allocating arguments and passing the extra command line, preventing
the default event from being incorrectly passed to the external command.

> diff --git a/tools/perf/builtin-kvm.c b/tools/perf/builtin-kvm.c
> index c236093eac473..acc6f2a706b35 100644
> --- a/tools/perf/builtin-kvm.c
> +++ b/tools/perf/builtin-kvm.c
> @@ -1996,13 +1996,14 @@ static int __cmd_record(const char *file_name, int argc, const char **argv)
>  {
>  	int rec_argc, i = 0, j, ret;
>  	const char **rec_argv;
> +	int need_arch_event = !!kvm_need_default_arch_event(argc, argv);
>  
>  	/*
>  	 * Besides the 2 more options "-o" and "filename",
>  	 * kvm_add_default_arch_event() may add 2 extra options,
> -	 * so allocate 4 more items.
> +	 * so allocate more items conditionally.
>  	 */
> -	rec_argc = argc + 2 + 2;
> +	rec_argc = argc + 2 + (2 * need_arch_event);

[ ... ]

> -	if (kvm_need_default_arch_event(argc, argv)) {
> +	if (need_arch_event) {
>  		ret = kvm_add_default_arch_event(EM_HOST, &i, rec_argv);
>  		if (ret)
>  			goto EXIT;
>  	}
>  
> +	for (j = 1; j < argc; j++, i++)
> +		rec_argv[i] = STRDUP_FAIL_EXIT(argv[j]);
> +
> -	BUG_ON(i + 2 != rec_argc);
> +	BUG_ON(i != rec_argc);

[Severity: High]
Could this BUG_ON() cause a regression for perf kvm record on architectures
that do not add default events?

If kvm_need_default_arch_event() returns true, rec_argc is set to argc + 4.

However, on architectures like ARM64, RISC-V, or s390, it appears
kvm_add_default_arch_event() can return 0 without actually appending any
events or incrementing i.

If that happens, i will only equal argc + 2 after the loop, causing this
assertion to unconditionally fail and abort the process.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260623070313.55225-1-namhyung@kernel.org?part=2