Re: [PATCH v3 03/27] KVM, vfio: remove symbol_get(kvm_put_kvm) from vfio

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Critical] Unlocking `kvm->lock` after calling `kvm_put_kvm()` results in a Use-After-Free if it was the last reference.
--

commit 79e5304c707c4ed618be6e55a950250edf9a7337
Author: Paolo Bonzini <pbonzini@redhat.com>

KVM, vfio: remove symbol_get(kvm_put_kvm) from vfio

This patch replaces the symbol_get() approach for KVM/VFIO interaction
by storing the address of kvm_destroy_vm in "struct kvm" to handle
kvm_put_kvm calls. This allows removing the cross-module dependencies
and directs kvm_put_kvm to the correct architecture implementation.

[ ... ]

> diff --git a/include/linux/kvm_types.h b/include/linux/kvm_types.h
> index 4cb68c71a13cd..8a9be07cb8dd6 100644
> --- a/include/linux/kvm_types.h
> +++ b/include/linux/kvm_types.h

[ ... ]

> @@ -162,6 +163,13 @@ static inline bool kvm_get_kvm_safe(struct kvm *kvm)
>  	return refcount_inc_not_zero(&rc->users_count);
>  }
>  
> +static inline void kvm_put_kvm(struct kvm *kvm)
> +{
> +	struct kvm_refcount *rc = (struct kvm_refcount *)kvm;
> +	if (refcount_dec_and_test(&rc->users_count))
> +		rc->destroy(kvm);
> +}

[Severity: Critical]
This is a pre-existing issue, but can calling kvm_put_kvm() here lead to a
use-after-free in vfio_ap_mdev_unset_kvm()?

Looking at vfio_ap_mdev_unset_kvm() in drivers/s390/crypto/vfio_ap_ops.c, it
calls get_update_locks_for_kvm(kvm) which acquires mutex_lock(&kvm->lock).

It then calls kvm_put_kvm(kvm) which decrements the reference count and
can trigger kvm_destroy_vm(), completely freeing the kvm struct memory:

drivers/s390/crypto/vfio_ap_ops.c:vfio_ap_mdev_unset_kvm() {
    ...
		get_update_locks_for_kvm(kvm);

		kvm_arch_crypto_clear_masks(kvm);
		vfio_ap_mdev_reset_queues(matrix_mdev);
		kvm_put_kvm(kvm);
		matrix_mdev->kvm = NULL;

		release_update_locks_for_kvm(kvm);
    ...
}

If the KVM file descriptor has already been closed by userspace, this could be
the final reference to the KVM object. When release_update_locks_for_kvm(kvm)
is subsequently called, it will attempt to execute mutex_unlock(&kvm->lock)
on the freed kvm pointer.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260529155050.2902245-1-seiden@linux.ibm.com?part=3