diff options
| author | Jason Gunthorpe <jgg@nvidia.com> | 2022-05-17 02:41:17 +0300 |
|---|---|---|
| committer | Alex Williamson <alex.williamson@redhat.com> | 2022-05-17 22:07:09 +0300 |
| commit | be8d3adae65cd44b6c299b796a5e1a0c24c54454 (patch) | |
| tree | 3c1115902cc3e83ffb42c4303add5c80d3058d95 /drivers/vfio/vfio.c | |
| parent | 6b17ca8e5e7a7b10689867dff5e22d7da368ba76 (diff) | |
| download | linux-be8d3adae65cd44b6c299b796a5e1a0c24c54454.tar.xz | |
vfio: Add missing locking for struct vfio_group::kvm
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>
Diffstat (limited to 'drivers/vfio/vfio.c')
| -rw-r--r-- | drivers/vfio/vfio.c | 19 |
1 files 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, |