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

[Namhyung Kim <namhyung@kernel.org>]

On Tue, Jun 23, 2026 at 07:16:57AM +0000, sashiko-bot@kernel.org wrote:
> 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.

I'll check those arch and make sure they return 0 in
kvm_need_default_arch_event().

Thanks,
Namhyung