Re: [RFC PATCH v2 11/15] KVM: arm64: Export stage-2 MMU config

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] cleanup: mixing goto-based cleanup and guard() in kvm_arch_get_guest_mmu()
--

commit 857dcf037db9acd0dfd4486a2d4b54b73644b090
Author: Jean-Philippe Brucker <jpb@kernel.org>

KVM: arm64: Export stage-2 MMU config

This commit provides an API to drivers that want to share the stage 2
configuration of a VM with their device. It introduces the
kvm_arch_get_guest_mmu and kvm_arch_put_guest_mmu functions to expose the
stage 2 MMU config of an EL1 VM.

> diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
> index 9ba86450fe4af..56490226f973b 100644
> --- a/arch/arm64/kvm/mmu.c
> +++ b/arch/arm64/kvm/mmu.c
[ ... ]
> +struct kvm_arch_guest_mmu *kvm_arch_get_guest_mmu(struct kvm *kvm)
> +{
> +	int ret;
> +	struct kvm_arch_guest_mmu *mmu;
> +
> +	guard(mutex)(&kvm->arch.config_lock);
> +
> +	if (is_protected_kvm_enabled())
> +		return ERR_PTR(-EOPNOTSUPP);
[ ... ]
> +	ret = kvm_arm_pinned_vmid_get(kvm);
> +	if (ret < 0)
> +		return ERR_PTR(ret);
> +
> +	ret = -ENOMEM;
> +	mmu = kzalloc_obj(*mmu, GFP_KERNEL_ACCOUNT);
> +	if (!mmu)
> +		goto err_unpin_vmid;
> +
> +	mmu->vtcr = kvm->arch.mmu.vtcr;
> +	mmu->vttbr = kvm_get_vttbr(&kvm->arch.mmu);
> +	mmu->hcr = kvm_mmu_get_hcr(kvm, 0);
> +
> +	return mmu;
> +
> +err_unpin_vmid:
> +	kvm_arm_pinned_vmid_put(kvm);
> +	return ERR_PTR(ret);
> +}

[Severity: Low]
Is it intentional to mix goto-based cleanup and guard macros in this function?

The function uses guard(mutex)(&kvm->arch.config_lock) for scope-based
locking, but also relies on the goto err_unpin_vmid label to clean up the
VMID when kzalloc_obj fails. 

According to the cleanup helpers guidelines, mixing goto-based error handling
with scope-based cleanup helpers in the same function creates confusing
ownership semantics and increases the risk of double-free or resource leak
bugs during future code edits.

Would it be better to convert this entirely to scope-based cleanup, or
alternatively, use manual unlocks without the guard macro?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261015142552.4128028-1-ryan.roberts@arm.com?part=11