Skip to content

Commit be8d3ad

Browse files
jgunthorpeawilliam
authored andcommitted
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: ccd46db ("vfio: support notifier chain in vfio_group") Reviewed-by: Kevin Tian <[email protected]> Signed-off-by: Jason Gunthorpe <[email protected]> Tested-by: Nicolin Chen <[email protected]> Tested-by: Matthew Rosato <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Alex Williamson <[email protected]>
1 parent 6b17ca8 commit be8d3ad

File tree

1 file changed

+15
-4
lines changed

1 file changed

+15
-4
lines changed

drivers/vfio/vfio.c

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ struct vfio_group {
7676
atomic_t opened;
7777
enum vfio_group_type type;
7878
unsigned int dev_counter;
79+
struct rw_semaphore group_rwsem;
7980
struct kvm *kvm;
8081
struct blocking_notifier_head notifier;
8182
};
@@ -360,6 +361,7 @@ static struct vfio_group *vfio_group_alloc(struct iommu_group *iommu_group,
360361
group->cdev.owner = THIS_MODULE;
361362

362363
refcount_set(&group->users, 1);
364+
init_rwsem(&group->group_rwsem);
363365
INIT_LIST_HEAD(&group->device_list);
364366
mutex_init(&group->device_lock);
365367
group->iommu_group = iommu_group;
@@ -1694,9 +1696,11 @@ void vfio_file_set_kvm(struct file *file, struct kvm *kvm)
16941696
if (file->f_op != &vfio_group_fops)
16951697
return;
16961698

1699+
down_write(&group->group_rwsem);
16971700
group->kvm = kvm;
16981701
blocking_notifier_call_chain(&group->notifier,
16991702
VFIO_GROUP_NOTIFY_SET_KVM, kvm);
1703+
up_write(&group->group_rwsem);
17001704
}
17011705
EXPORT_SYMBOL_GPL(vfio_file_set_kvm);
17021706

@@ -2004,15 +2008,22 @@ static int vfio_register_group_notifier(struct vfio_group *group,
20042008
return -EINVAL;
20052009

20062010
ret = blocking_notifier_chain_register(&group->notifier, nb);
2011+
if (ret)
2012+
return ret;
20072013

20082014
/*
20092015
* The attaching of kvm and vfio_group might already happen, so
20102016
* here we replay once upon registration.
20112017
*/
2012-
if (!ret && set_kvm && group->kvm)
2013-
blocking_notifier_call_chain(&group->notifier,
2014-
VFIO_GROUP_NOTIFY_SET_KVM, group->kvm);
2015-
return ret;
2018+
if (set_kvm) {
2019+
down_read(&group->group_rwsem);
2020+
if (group->kvm)
2021+
blocking_notifier_call_chain(&group->notifier,
2022+
VFIO_GROUP_NOTIFY_SET_KVM,
2023+
group->kvm);
2024+
up_read(&group->group_rwsem);
2025+
}
2026+
return 0;
20162027
}
20172028

20182029
int vfio_register_notifier(struct vfio_device *device,

0 commit comments

Comments
 (0)