viommu_remove() resets the device and deletes the virtqueues while
userspace may still hold domains that it maps or attaches, and nothing
stopped a request from being queued - or a drain from walking the queue -
on a virtqueue that was being torn down.
Set ->removed under request_lock before the teardown - the lock a
queueing path holds while it checks liveness and adds the request, and
the one the drain holds for its whole run - check it in the queueing
helper, and let the drain return early when it is set.
Fixes: edcd69ab9a32 ("iommu: Add virtio-iommu driver")
Signed-off-by: Anlai Lu <[email protected]>
---
drivers/iommu/virtio-iommu.c | 32 ++++++++++++++++++++++++++++++++
1 file changed, 32 insertions(+)
diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
index 1a4366458751..ba1d78e4daa0 100644
--- a/drivers/iommu/virtio-iommu.c
+++ b/drivers/iommu/virtio-iommu.c
@@ -42,6 +42,11 @@ struct viommu_dev {
spinlock_t request_lock;
struct list_head requests;
void *evts;
+ /*
+ * Set before the teardown: nothing may be queued or drained after
+ * that.
+ */
+ bool removed;
/* Device configuration */
struct iommu_domain_geometry geometry;
@@ -116,6 +121,16 @@ static struct viommu_domain viommu_identity_domain;
#define to_viommu_domain(domain) \
container_of(domain, struct viommu_domain, domain)
+/*
+ * The device can be removed while its domains still exist (userspace may hold
+ * them for a while), and the queues go away with it: nothing may be queued or
+ * drained after that.
+ */
+static bool viommu_device_live(struct viommu_dev *viommu)
+{
+ return !viommu->removed;
+}
+
static int viommu_get_req_errno(void *buf, size_t len)
{
struct virtio_iommu_req_tail *tail = buf + len - sizeof(*tail);
@@ -206,6 +221,10 @@ static int viommu_sync_req(struct viommu_dev *viommu)
unsigned long flags;
spin_lock_irqsave(&viommu->request_lock, flags);
+ if (!viommu_device_live(viommu)) {
+ spin_unlock_irqrestore(&viommu->request_lock, flags);
+ return 0;
+ }
ret = __viommu_sync_req(viommu);
if (ret)
dev_dbg(viommu->dev, "could not sync requests (%d)\n", ret);
@@ -229,6 +248,9 @@ static int __viommu_queue_req(struct viommu_dev *viommu,
assert_spin_locked(&viommu->request_lock);
+ if (!viommu_device_live(viommu))
+ return -ENODEV;
+
sg_init_one(&top_sg, req->buf, write_offset);
sg_init_one(&bottom_sg, req->buf + write_offset,
req->len - write_offset);
@@ -1555,10 +1577,20 @@ static int viommu_probe(struct virtio_device *vdev)
static void viommu_remove(struct virtio_device *vdev)
{
struct viommu_dev *viommu = vdev->priv;
+ unsigned long flags;
iommu_device_sysfs_remove(&viommu->iommu);
iommu_device_unregister(&viommu->iommu);
+ /*
+ * The queues go away here: nothing may be queued or drained from now
+ * on. Taking request_lock is what tells a drain in flight that it has
+ * to finish before the teardown, since the drain holds it throughout.
+ */
+ spin_lock_irqsave(&viommu->request_lock, flags);
+ viommu->removed = true;
+ spin_unlock_irqrestore(&viommu->request_lock, flags);
+
/* Stop all virtqueues */
virtio_reset_device(vdev);
vdev->config->del_vqs(vdev);
--
2.55.0
