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 <[email protected]>

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/[email protected]?part=11

Reply via email to