Re: [PATCH kvmtool v2] arm64: Fix resource leaks in find_pmu_cpumask()

[Alexandru Elisei <alexandru.elisei@arm.com>]

Hi,

On Thu, Aug 06, 2026 at 10:32:18AM +0800, Zongmin Zhou wrote:
> From: Zongmin Zhou <zhouzongmin@kylinos.cn>
> 
> Close file descriptors on read_file() failure and close the directory
> stream before returning from the function.
> 
> Signed-off-by: Zongmin Zhou <zhouzongmin@kylinos.cn>
> ---
> Changes since v1:
> 
> Save errno before close() so a close() failure doesn't clobber it,
> allowing a single close() for both paths.
> ---
>  arm64/pmu.c | 14 +++++++++-----
>  1 file changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/arm64/pmu.c b/arm64/pmu.c
> index 78c15f1..ef7faca 100644
> --- a/arm64/pmu.c
> +++ b/arm64/pmu.c
> @@ -75,7 +75,7 @@ static int find_pmu_cpumask(struct kvm *kvm, cpumask_t *cpumask)
>  	unsigned long val;
>  	ssize_t fd_sz;
>  	int fd, ret;
> -	DIR *dir;
> +	DIR *dir = NULL;
>  
>  	memset(buf, 0, sizeof(buf));
>  
> @@ -107,11 +107,12 @@ static int find_pmu_cpumask(struct kvm *kvm, cpumask_t *cpumask)
>  			goto next_dir;
>  
>  		fd_sz = read_file(fd, cpulist, PAGE_SIZE);
> +		ret = errno;
> +		close(fd);
>  		if (fd_sz < 0) {
> -			pmu_id = -errno;
> +			pmu_id = -ret;
>  			goto out_free;
>  		}
> -		close(fd);
>  
>  		ret = cpulist_parse(cpulist, &pmu_cpumask);
>  		if (ret) {
> @@ -140,11 +141,12 @@ static int find_pmu_cpumask(struct kvm *kvm, cpumask_t *cpumask)
>  			goto next_dir;
>  
>  		fd_sz = read_file(fd, buf, PMU_ID_MAXLEN - 1);
> +		ret = errno;
> +		close(fd);
>  		if (fd_sz < 0) {
> -			pmu_id = -errno;
> +			pmu_id = -ret;
>  			goto out_free;
>  		}
> -		close(fd);
>  
>  		val = strtoul(buf, NULL, 10);
>  		if (val > INT_MAX) {
> @@ -162,6 +164,8 @@ next_dir:
>  	}
>  
>  out_free:
> +	if (dir)
> +		closedir(dir);
>  	free(path);
>  	free(cpulist);
>  	return pmu_id;

Looks good to me:

Reviewed-by: Alexandru Elisei <alexandru.elisei@arm.com>

Just one tiny, tiny nitpick: I would have set ret = -errno so we can do
pmu_id = ret in case of error, like we do for cpulist_parse() errors, but that's
totally unimportant.

Thanks,
Alex