Re: [PATCH] KVM: Make kvm_io_bus_get_dev() filter devices by ops

[Marc Zyngier <maz@kernel.org>]

On Fri, 26 Jun 2026 12:46:35 +0100,
sashiko-bot@kernel.org wrote:
> 
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] Dropping the SRCU lock in kvm_io_bus_get_dev() exposes a Use-After-Free for VGIC redistributors.
> --
> 
> commit 954101cd7bbd1642133e68638e5210cf89b88023
> Author: Marc Zyngier <maz@kernel.org>
> 
> KVM: Make kvm_io_bus_get_dev() filter devices by ops
> 
> This commit updates kvm_io_bus_get_dev() to check the device ops pointer
> before dropping the SRCU lock. This aims to prevent lifetime issues where
> a caller might introspect an object of the wrong type after the lock is
> dropped.
> 
> > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> > --- a/virt/kvm/kvm_main.c
> > +++ b/virt/kvm/kvm_main.c
> 
> [ ... ]
> 
> > @@ -6081,7 +6082,9 @@ struct kvm_io_device *kvm_io_bus_get_dev(struct kvm *kvm, enum kvm_bus bus_idx,
> >  	if (dev_idx < 0)
> >  		goto out_unlock;
> >  
> > -	iodev = bus->range[dev_idx].dev;
> > +	if (bus->range[dev_idx].dev &&
> > +	    bus->range[dev_idx].dev->ops == ops)
> > +		iodev = bus->range[dev_idx].dev;
> >  
> >  out_unlock:
> >  	srcu_read_unlock(&kvm->srcu, srcu_idx);
> 
> [Severity: High]
> This is a pre-existing issue, but does dropping the SRCU lock here before
> returning the matched device pointer expose a use-after-free?
> 
> If a host VMM concurrently executes KVM_SIGNAL_MSI (with a doorbell address
> overlapping the redistributor) and KVM_CREATE_VCPU (forced to fail due to
> resource exhaustion), the failed vCPU creation path can unregister the
> device and free the vCPU memory.
> 
> Because the SRCU lock is already dropped here, the unregister path won't
> be blocked. When the MSI injection path resumes in __vgic_doorbell_to_its(),
> it can dereference the freed vCPU memory:
> 
> arch/arm64/kvm/vgic/vgic-its.c:__vgic_doorbell_to_its() {
>     ...
>     kvm_io_dev = kvm_io_bus_get_dev(kvm, KVM_MMIO_BUS, &kvm_io_gic_ops, db);
>     if (!kvm_io_dev)
>         return ERR_PTR(-EINVAL);
> 
>     iodev = container_of(kvm_io_dev, struct vgic_io_device, dev);
>     if (iodev->iodev_type != IODEV_ITS)
>         return ERR_PTR(-EINVAL);
>     ...
> }

Gniii... I guess the last option is to bite the bullet and make sure
that any filtering occurs with the SRCU lock held. Something like the
patch below. Or just move the locking outside of the helper and keep
it completely inside the caller.

	M.

From 4f6872cf57b3d30dcc81155e0ef6ac8bc6075fba Mon Sep 17 00:00:00 2001
From: Marc Zyngier <maz@kernel.org>
Date: Thu, 25 Jun 2026 15:28:38 +0100
Subject: [PATCH] KVM: Add lock-protected filtering to kvm_io_bus_get_dev()

kvm_io_bus_get_dev() returns a device that is only matched by the
address, and nothing else. This can cause a lifetime issue if
the matched device is not the expected type, as by the time
the caller can introspect the object, it might be gone (the srcu
lock having been dropped).

Add a callback to this helper so that users can check the validity
of the device while under the srcu lock, and update the sole user
to pass its own filter.

Reported-by: Will Deacon <will@kernel.org>
Signed-off-by: Marc Zyngier <maz@kernel.org>
Fixes: 8a39d00670f07 ("KVM: kvm_io_bus: Add kvm_io_bus_get_dev() call")
Cc: stable@vger.kernel.org
---
 arch/arm64/kvm/vgic/vgic-its.c | 24 ++++++++++++++----------
 include/linux/kvm_host.h       |  1 +
 virt/kvm/kvm_main.c            |  4 +++-
 3 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/arch/arm64/kvm/vgic/vgic-its.c b/arch/arm64/kvm/vgic/vgic-its.c
index 4477f870c7b36..d8541690e7ba2 100644
--- a/arch/arm64/kvm/vgic/vgic-its.c
+++ b/arch/arm64/kvm/vgic/vgic-its.c
@@ -503,23 +503,27 @@ static unsigned long vgic_mmio_read_its_idregs(struct kvm *kvm,
 	return 0;
 }
 
+static bool vgic_its_device_filter(struct kvm_io_device *dev)
+{
+	if (dev && dev->ops == &kvm_io_gic_ops) {
+		struct vgic_io_device *iodev;
+
+		iodev = container_of(dev, struct vgic_io_device, dev);
+		return iodev->iodev_type == IODEV_ITS;
+	}
+
+	return false;
+}
+
 static struct vgic_its *__vgic_doorbell_to_its(struct kvm *kvm, gpa_t db)
 {
 	struct kvm_io_device *kvm_io_dev;
-	struct vgic_io_device *iodev;
 
-	kvm_io_dev = kvm_io_bus_get_dev(kvm, KVM_MMIO_BUS, db);
+	kvm_io_dev = kvm_io_bus_get_dev(kvm, KVM_MMIO_BUS, vgic_its_device_filter, db);
 	if (!kvm_io_dev)
 		return ERR_PTR(-EINVAL);
 
-	if (kvm_io_dev->ops != &kvm_io_gic_ops)
-		return ERR_PTR(-EINVAL);
-
-	iodev = container_of(kvm_io_dev, struct vgic_io_device, dev);
-	if (iodev->iodev_type != IODEV_ITS)
-		return ERR_PTR(-EINVAL);
-
-	return iodev->its;
+	return container_of(kvm_io_dev, struct vgic_io_device, dev)->its;
 }
 
 static unsigned long vgic_its_cache_key(u32 devid, u32 eventid)
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 4c14aee1fb063..3610a6986be3d 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -231,6 +231,7 @@ int kvm_io_bus_register_dev(struct kvm *kvm, enum kvm_bus bus_idx, gpa_t addr,
 int kvm_io_bus_unregister_dev(struct kvm *kvm, enum kvm_bus bus_idx,
 			      struct kvm_io_device *dev);
 struct kvm_io_device *kvm_io_bus_get_dev(struct kvm *kvm, enum kvm_bus bus_idx,
+					 bool (*filter)(struct kvm_io_device *),
 					 gpa_t addr);
 
 #ifdef CONFIG_KVM_ASYNC_PF
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 881f92d7a469e..c4f6044ae89fe 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -6066,6 +6066,7 @@ int kvm_io_bus_unregister_dev(struct kvm *kvm, enum kvm_bus bus_idx,
 }
 
 struct kvm_io_device *kvm_io_bus_get_dev(struct kvm *kvm, enum kvm_bus bus_idx,
+					 bool (*filter)(struct kvm_io_device *),
 					 gpa_t addr)
 {
 	struct kvm_io_bus *bus;
@@ -6082,7 +6083,8 @@ struct kvm_io_device *kvm_io_bus_get_dev(struct kvm *kvm, enum kvm_bus bus_idx,
 	if (dev_idx < 0)
 		goto out_unlock;
 
-	iodev = bus->range[dev_idx].dev;
+	if (filter(bus->range[dev_idx].dev))
+		iodev = bus->range[dev_idx].dev;
 
 out_unlock:
 	srcu_read_unlock(&kvm->srcu, srcu_idx);
-- 
2.47.3


-- 
Without deviation from the norm, progress is not possible.