Re: [PATCH] vduse: Fix race condition between resetting and irq injecting

Started by Jason Wang on Sep 28, 2021

open the first message on its own page

Message details and related content
On Mon, Sep 27, 2021 at 9:18 PM Xie Yongji [off-list ref] wrote:
quoted hunk
The interrupt might be triggered after a reset since there is
no synchronization between resetting and irq injecting. And it
might break something if the interrupt is delayed until a new
round of device initialization.

Fixes: c8a6153b6c59 ("vduse: Introduce VDUSE - vDPA Device in Userspace")
Signed-off-by: Xie Yongji <redacted>
---
 drivers/vdpa/vdpa_user/vduse_dev.c | 31 +++++++++++++++++++------------
 1 file changed, 19 insertions(+), 12 deletions(-)
diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
index cefb301b2ee4..ea7d80ade559 100644
--- a/drivers/vdpa/vdpa_user/vduse_dev.c
+++ b/drivers/vdpa/vdpa_user/vduse_dev.c
@@ -80,6 +80,7 @@ struct vduse_dev {
        struct vdpa_callback config_cb;
        struct work_struct inject;
        spinlock_t irq_lock;
+       struct rw_semaphore rwsem;
        int minor;
        bool broken;
        bool connected;
@@ -410,6 +411,8 @@ static void vduse_dev_reset(struct vduse_dev *dev)
        if (domain->bounce_map)
                vduse_domain_reset_bounce_map(domain);

+       down_write(&dev->rwsem);
+
        dev->status = 0;
        dev->driver_features = 0;
        dev->generation++;
@@ -443,6 +446,8 @@ static void vduse_dev_reset(struct vduse_dev *dev)
                flush_work(&vq->inject);
                flush_work(&vq->kick);
        }
+
+       up_write(&dev->rwsem);
 }

 static int vduse_vdpa_set_vq_address(struct vdpa_device *vdpa, u16 idx,
@@ -967,11 +972,12 @@ static long vduse_dev_ioctl(struct file *file, unsigned int cmd,
        }
        case VDUSE_DEV_INJECT_CONFIG_IRQ:
                ret = -EINVAL;
-               if (!(dev->status & VIRTIO_CONFIG_S_DRIVER_OK))
-                       break;
-
-               ret = 0;
-               queue_work(vduse_irq_wq, &dev->inject);
+               down_read(&dev->rwsem);
+               if (dev->status & VIRTIO_CONFIG_S_DRIVER_OK) {
+                       ret = 0;
+                       queue_work(vduse_irq_wq, &dev->inject);
+               }
+               up_read(&dev->rwsem);
                break;
        case VDUSE_VQ_SETUP: {
                struct vduse_vq_config config;
@@ -1049,10 +1055,6 @@ static long vduse_dev_ioctl(struct file *file, unsigned int cmd,
        case VDUSE_VQ_INJECT_IRQ: {
                u32 index;

-               ret = -EINVAL;
-               if (!(dev->status & VIRTIO_CONFIG_S_DRIVER_OK))
-                       break;
-
                ret = -EFAULT;
                if (get_user(index, (u32 __user *)argp))
                        break;
@@ -1061,9 +1063,13 @@ static long vduse_dev_ioctl(struct file *file, unsigned int cmd,
                if (index >= dev->vq_num)
                        break;

-               ret = 0;
-               index = array_index_nospec(index, dev->vq_num);
-               queue_work(vduse_irq_wq, &dev->vqs[index].inject);
+               down_read(&dev->rwsem);
+               if (dev->status & VIRTIO_CONFIG_S_DRIVER_OK) {
+                       ret = 0;
+                       index = array_index_nospec(index, dev->vq_num);
+                       queue_work(vduse_irq_wq, &dev->vqs[index].inject);
+               }
+               up_read(&dev->rwsem);
Nit: I think we can factor out the common logic to a helper.

Others look good.

Thanks
quoted hunk
                break;
        }
        default:
@@ -1144,6 +1150,7 @@ static struct vduse_dev *vduse_dev_create(void)
        INIT_LIST_HEAD(&dev->send_list);
        INIT_LIST_HEAD(&dev->recv_list);
        spin_lock_init(&dev->irq_lock);
+       init_rwsem(&dev->rwsem);

        INIT_WORK(&dev->inject, vduse_dev_irq_inject);
        init_waitqueue_head(&dev->waitq);
--
2.11.0
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Keyboard shortcuts
hback out one level
jnext message in thread / next series-diff position
kprevious message in thread / previous series-diff position
ldrill in
Escclose help / fold thread tree
?toggle this help