[PATCH] s390/vfio-ap: fix stale pqap_hook pointer on error in vfio_ap_mdev_set_kvm()

[PATCH] s390/vfio-ap: fix stale pqap_hook pointer on error in vfio_ap_mdev_set_kvm()

[Anthony Krowiak]

In vfio_ap_mdev_set_kvm(), kvm->arch.crypto.pqap_hook is set to
&matrix_mdev->pqap_hook before the update locks are acquired and the
mdev list is checked for a conflicting assignment. If another mdev is
already attached to the same KVM instance, the function returns -EPERM
without restoring the hook pointer, leaving kvm->arch.crypto.pqap_hook
pointing at the failing matrix_mdev instead of the mdev that legitimately
owns the KVM.

Since matrix_mdev->kvm is never set on this error path,
vfio_ap_mdev_unset_kvm() will not clean up the hook when matrix_mdev
is later closed. If matrix_mdev is subsequently freed, any PQAP
instruction executed by the guest will dereference the stale pointer
through pqap_hook_rwsem, resulting in a use-after-free.

Fix this by restoring kvm->arch.crypto.pqap_hook to &m->pqap_hook on
the -EPERM path, where m is the already-attached mdev discovered during
the conflict check. This is the value the hook held before it was
overwritten, and it is the only safe value to restore since m remains
live and its pqap_hook is the handler the guest must continue to use.

The rwsem is re-taken under write before restoring the pointer, which
is consistent with the existing locking pattern and ensures no VCPU
thread can observe a torn value.

Fixes: 86956e70761b3 ("s390/vfio-ap: replace open coded locks for VFIO_GROUP_NOTIFY_SET_KVM notification")
Cc: stable@vger.kernel.org

Signed-off-by: Anthony Krowiak <akrowiak@linux.ibm.com>
---
 drivers/s390/crypto/vfio_ap_ops.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c
index 44b3a1dcc1b3..e18540fd6dc7 100644
--- a/drivers/s390/crypto/vfio_ap_ops.c
+++ b/drivers/s390/crypto/vfio_ap_ops.c
@@ -1824,6 +1824,12 @@ static int vfio_ap_mdev_set_kvm(struct ap_matrix_mdev *matrix_mdev,
 	struct ap_matrix_mdev *m;
 
 	if (kvm->arch.crypto.crycbd) {
+		/*
+		 * Taking the kvm->arch.crypto.pqap_hook_rwsem lock while
+		 * holding the update locks (below) could result in a
+		 * deadlock condition, so let's hold the rwsem here while we
+		 * update the pqap_hook.
+		 */
 		down_write(&kvm->arch.crypto.pqap_hook_rwsem);
 		kvm->arch.crypto.pqap_hook = &matrix_mdev->pqap_hook;
 		up_write(&kvm->arch.crypto.pqap_hook_rwsem);
@@ -1833,6 +1839,10 @@ static int vfio_ap_mdev_set_kvm(struct ap_matrix_mdev *matrix_mdev,
 		list_for_each_entry(m, &matrix_dev->mdev_list, node) {
 			if (m != matrix_mdev && m->kvm == kvm) {
 				release_update_locks_for_kvm(kvm);
+				/* Undo the hook installation since we're failing */
+				down_write(&kvm->arch.crypto.pqap_hook_rwsem);
+				kvm->arch.crypto.pqap_hook = &m->pqap_hook;
+				up_write(&kvm->arch.crypto.pqap_hook_rwsem);
 				return -EPERM;
 			}
 		}
-- 
2.53.0