On Wed, 10 Feb 2021 16:24:29 +0100 Halil Pasic <pa...@linux.ibm.com> wrote:
> > Maybe you could > > - 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? > > I agree, matrix_mdev->kvm can go NULL any time and we are risking > 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; s/matrix_mdev->kvm/kvm > vfio_ap_mdev_reset_queues(matrix_mdev->mdev); > > 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. But the problem with that is that we enable the the assign/unassign prematurely, which could interfere wit reset_queues(). Forget about it.