On Wed, 10 Feb 2021 16:24:29 +0100
Halil Pasic <pasic@xxxxxxxxxxxxx> wrote:
s/matrix_mdev->kvm/kvmMaybe you couldI agree, matrix_mdev->kvm can go NULL any time and we are risking
- grab a reference to kvm while holding the lock
- call the mask handling functions with that kvm reference
- lock again, drop the reference, and do the rest of the processing?
a null pointer dereference here.
Another idea would be to do
static void vfio_ap_mdev_unset_kvm(struct ap_matrix_mdev *matrix_mdev)
{
struct kvm *kvm;
mutex_lock(&matrix_dev->lock);
if (matrix_mdev->kvm) {
kvm = matrix_mdev->kvm;
matrix_mdev->kvm = NULL;
mutex_unlock(&matrix_dev->lock);
kvm_arch_crypto_clear_masks(kvm);
mutex_lock(&matrix_dev->lock);
matrix_mdev->kvm->arch.crypto.pqap_hook = NULL;
vfio_ap_mdev_reset_queues(matrix_mdev->mdev);But the problem with that is that we enable the the assign/unassign
kvm_put_kvm(kvm);
}
mutex_unlock(&matrix_dev->lock);
}
That way only one unset would actually do the unset and cleanup
and every other invocation would bail out with only checking
matrix_mdev->kvm.
prematurely, which could interfere wit reset_queues(). Forget about
it.