From: Jason Gunthorpe <jgg@nvidia.com>
Date: Mon, 16 May 2022 20:41:17 -0300
Subject: vfio: Add missing locking for struct vfio_group::kvm
Git-commit: be8d3adae65cd44b6c299b796a5e1a0c24c54454
Patch-mainline: v5.19-rc1
References: bsc#1205701
Without locking userspace can trigger a UAF by racing
KVM_DEV_VFIO_GROUP_DEL with VFIO_GROUP_GET_DEVICE_FD:
CPU1 CPU2
ioctl(KVM_DEV_VFIO_GROUP_DEL)
ioctl(VFIO_GROUP_GET_DEVICE_FD)
vfio_group_get_device_fd
open_device()
intel_vgpu_open_device()
vfio_register_notifier()
vfio_register_group_notifier()
blocking_notifier_call_chain(&group->notifier,
VFIO_GROUP_NOTIFY_SET_KVM, group->kvm);
set_kvm()
group->kvm = NULL
close()
kfree(kvm)
intel_vgpu_group_notifier()
vdev->kvm = data
[..]
kvm_get_kvm(vgpu->kvm);
// UAF!
Add a simple rwsem in the group to protect the kvm while the notifier is
using it.
Note this doesn't fix the race internal to i915 where userspace can
trigger two VFIO_GROUP_NOTIFY_SET_KVM's before we reach a consumer of
vgpu->kvm and trigger this same UAF, it just makes the notifier
self-consistent.
Fixes: ccd46dbae77d ("vfio: support notifier chain in vfio_group")
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
Tested-by: Nicolin Chen <nicolinc@nvidia.com>
Tested-by: Matthew Rosato <mjrosato@linux.ibm.com>
Link: https://lore.kernel.org/r/1-v2-d035a1842d81+1bf-vfio_group_locking_jgg@nvidia.com
Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
Acked-by: Joerg Roedel <jroedel@suse.de>
---
drivers/vfio/vfio.c | 19 +++++++++++++++----
1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
index 1758d96f43f4..4261eeec9e73 100644
--- a/drivers/vfio/vfio.c
+++ b/drivers/vfio/vfio.c
@@ -76,6 +76,7 @@ struct vfio_group {
atomic_t opened;
enum vfio_group_type type;
unsigned int dev_counter;
+ struct rw_semaphore group_rwsem;
struct kvm *kvm;
struct blocking_notifier_head notifier;
};
@@ -360,6 +361,7 @@ static struct vfio_group *vfio_group_alloc(struct iommu_group *iommu_group,
group->cdev.owner = THIS_MODULE;
refcount_set(&group->users, 1);
+ init_rwsem(&group->group_rwsem);
INIT_LIST_HEAD(&group->device_list);
mutex_init(&group->device_lock);
group->iommu_group = iommu_group;
@@ -1694,9 +1696,11 @@ void vfio_file_set_kvm(struct file *file, struct kvm *kvm)
if (file->f_op != &vfio_group_fops)
return;
+ down_write(&group->group_rwsem);
group->kvm = kvm;
blocking_notifier_call_chain(&group->notifier,
VFIO_GROUP_NOTIFY_SET_KVM, kvm);
+ up_write(&group->group_rwsem);
}
EXPORT_SYMBOL_GPL(vfio_file_set_kvm);
@@ -2004,15 +2008,22 @@ static int vfio_register_group_notifier(struct vfio_group *group,
return -EINVAL;
ret = blocking_notifier_chain_register(&group->notifier, nb);
+ if (ret)
+ return ret;
/*
* The attaching of kvm and vfio_group might already happen, so
* here we replay once upon registration.
*/
- if (!ret && set_kvm && group->kvm)
- blocking_notifier_call_chain(&group->notifier,
- VFIO_GROUP_NOTIFY_SET_KVM, group->kvm);
- return ret;
+ if (set_kvm) {
+ down_read(&group->group_rwsem);
+ if (group->kvm)
+ blocking_notifier_call_chain(&group->notifier,
+ VFIO_GROUP_NOTIFY_SET_KVM,
+ group->kvm);
+ up_read(&group->group_rwsem);
+ }
+ return 0;
}
int vfio_register_notifier(struct vfio_device *device,