Skip to content

Commit ea54dd3

Browse files
ouptonMarc Zyngier
authored andcommitted
KVM: Treat the device list as an rculist
A subsequent change to KVM/arm64 will necessitate walking the device list outside of the kvm->lock. Prepare by converting to an rculist. This has zero effect on the VM destruction path, as it is expected every reader is backed by a reference on the kvm struct. On the other hand, ensure a given device is completely destroyed before dropping the kvm->lock in the release() path, as certain devices expect to be a singleton (e.g. the vfio-kvm device). Cc: Paolo Bonzini <pbonzini@redhat.com> Cc: Sean Christopherson <seanjc@google.com> Signed-off-by: Oliver Upton <oliver.upton@linux.dev> Reviewed-by: Sean Christopherson <seanjc@google.com> Link: https://lore.kernel.org/r/20240422200158.2606761-2-oliver.upton@linux.dev Signed-off-by: Marc Zyngier <maz@kernel.org>
1 parent fec50db commit ea54dd3

2 files changed

Lines changed: 13 additions & 3 deletions

File tree

virt/kvm/kvm_main.c

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1329,6 +1329,12 @@ static void kvm_destroy_devices(struct kvm *kvm)
13291329
* We do not need to take the kvm->lock here, because nobody else
13301330
* has a reference to the struct kvm at this point and therefore
13311331
* cannot access the devices list anyhow.
1332+
*
1333+
* The device list is generally managed as an rculist, but list_del()
1334+
* is used intentionally here. If a bug in KVM introduced a reader that
1335+
* was not backed by a reference on the kvm struct, the hope is that
1336+
* it'd consume the poisoned forward pointer instead of suffering a
1337+
* use-after-free, even though this cannot be guaranteed.
13321338
*/
13331339
list_for_each_entry_safe(dev, tmp, &kvm->devices, vm_node) {
13341340
list_del(&dev->vm_node);
@@ -4725,7 +4731,8 @@ static int kvm_device_release(struct inode *inode, struct file *filp)
47254731

47264732
if (dev->ops->release) {
47274733
mutex_lock(&kvm->lock);
4728-
list_del(&dev->vm_node);
4734+
list_del_rcu(&dev->vm_node);
4735+
synchronize_rcu();
47294736
dev->ops->release(dev);
47304737
mutex_unlock(&kvm->lock);
47314738
}
@@ -4808,7 +4815,7 @@ static int kvm_ioctl_create_device(struct kvm *kvm,
48084815
kfree(dev);
48094816
return ret;
48104817
}
4811-
list_add(&dev->vm_node, &kvm->devices);
4818+
list_add_rcu(&dev->vm_node, &kvm->devices);
48124819
mutex_unlock(&kvm->lock);
48134820

48144821
if (ops->init)
@@ -4819,7 +4826,8 @@ static int kvm_ioctl_create_device(struct kvm *kvm,
48194826
if (ret < 0) {
48204827
kvm_put_kvm_no_destroy(kvm);
48214828
mutex_lock(&kvm->lock);
4822-
list_del(&dev->vm_node);
4829+
list_del_rcu(&dev->vm_node);
4830+
synchronize_rcu();
48234831
if (ops->release)
48244832
ops->release(dev);
48254833
mutex_unlock(&kvm->lock);

virt/kvm/vfio.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -366,6 +366,8 @@ static int kvm_vfio_create(struct kvm_device *dev, u32 type)
366366
struct kvm_device *tmp;
367367
struct kvm_vfio *kv;
368368

369+
lockdep_assert_held(&dev->kvm->lock);
370+
369371
/* Only one VFIO "device" per VM */
370372
list_for_each_entry(tmp, &dev->kvm->devices, vm_node)
371373
if (tmp->ops == &kvm_vfio_ops)

0 commit comments

Comments
 (0)