Re: [PATCH v2 4/4] perf tools: Fix double frees and memory leaks in cmd_kvm()

Thank you for your contribution! Sashiko AI review found 1 potential issue(=
s) to consider:

Pre-existing issues:
- [High] The patch fixes memory leaks and double-free vulnerabilities in `_=
_cmd_record`, `__cmd_report`, `__cmd_buildid_list`, and `__cmd_top` by remo=
ving `STRDUP_FAIL_EXIT` and the element-wise `free()` loop on `rec_argv`, b=
ut it misses applying the exact same fix to `kvm_events_record()` which suf=
fers from the identical bug.
--

--- Patch [4]: [PATCH v2 4/4] perf tools: Fix double frees and memory leaks=
 in cmd_kvm() ---
commit 0b9d8e748caa2edae90fe11689ed1a0bf618e63b
Author: Michalis Niarchos <[email protected]>

perf tools: Fix double frees and memory leaks in cmd_kvm()

This commit fixes double free and memory leak vulnerabilities in various
builtin KVM commands (like __cmd_record, __cmd_report, __cmd_buildid_list,
and __cmd_top). It achieves this by removing the use of STRDUP_FAIL_EXIT and
dropping the element-wise free loop on rec_argv.

> diff --git a/tools/perf/builtin-kvm.c b/tools/perf/builtin-kvm.c
> index 9504c83e20744..04cf9bd5b5959 100644
> --- a/tools/perf/builtin-kvm.c
> +++ b/tools/perf/builtin-kvm.c
> @@ -2018,8 +2018,6 @@ static int __cmd_record(const char *file_name, int =
argc, const char **argv)
>  	ret =3D cmd_record(i, rec_argv);
> =20
>  EXIT:
> -	for (i =3D 0; i < rec_argc; i++)
> -		free((void *)rec_argv[i]);
>  	free(rec_argv);
>  	return ret;
>  }

[Severity: High]
This is a pre-existing issue and was not introduced by this patch, but does
kvm_events_record() in tools/perf/builtin-kvm.c require this exact same fix?

The commit message notes that the fixed functions follow the same pattern
as kvm_events_record(). However, looking at kvm_events_record(), it appears
it still contains the flawed cleanup loop:

tools/perf/builtin-kvm.c:kvm_events_record() {
    ...
EXIT:
    for (i =3D 0; i < rec_argc; i++)
        free((void *)rec_argv[i]);
    free(rec_argv);
    return ret;
}

When a user runs 'perf kvm stat record', cmd_record() processes the argumen=
ts
by calling parse_options(). Since parse_options() shifts the array in-place
using memmove() to consume recognized arguments, iterating over the original
length of rec_argv (rec_argc) during cleanup encounters duplicated pointers=
 at
the end of the array.

Will leaving this loop in kvm_events_record() lead to a double-free memory
corruption when those duplicate pointers are freed?

--=20
Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260803-perf-kvm-f=
[email protected]?part=3D4