* Re: [PATCH] vduse: Disallow injecting interrupt before DRIVER_OK is set
[not found] <20210923075722.98-1-xieyongji@bytedance.com>
@ 2021-09-26 2:28 ` Jason Wang
2021-10-13 11:26 ` Michael S. Tsirkin
1 sibling, 0 replies; 2+ messages in thread
From: Jason Wang @ 2021-09-26 2:28 UTC (permalink / raw)
To: Xie Yongji; +Cc: virtualization, mst
On Thu, Sep 23, 2021 at 3:57 PM Xie Yongji <xieyongji@bytedance.com> wrote:
>
> The interrupt callback should not be triggered before DRIVER_OK
> is set. Otherwise, it might break the virtio device driver.
> So let's add a check to avoid the unexpected behavior.
>
> Fixes: c8a6153b6c59 ("vduse: Introduce VDUSE - vDPA Device in Userspace")
> Signed-off-by: Xie Yongji <xieyongji@bytedance.com>
> ---
> drivers/vdpa/vdpa_user/vduse_dev.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
> index 29a38ecba19e..972c13a7e5da 100644
> --- a/drivers/vdpa/vdpa_user/vduse_dev.c
> +++ b/drivers/vdpa/vdpa_user/vduse_dev.c
> @@ -968,6 +968,10 @@ static long vduse_dev_ioctl(struct file *file, unsigned int cmd,
> break;
> }
> case VDUSE_DEV_INJECT_CONFIG_IRQ:
> + ret = -EINVAL;
> + if (!(dev->status & VIRTIO_CONFIG_S_DRIVER_OK))
> + break;
> +
I wonder if we need any synchronization with set_status()?
Thanks
> ret = 0;
> queue_work(vduse_irq_wq, &dev->inject);
> break;
> @@ -1047,6 +1051,10 @@ 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;
> --
> 2.11.0
>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 2+ messages in thread* Re: [PATCH] vduse: Disallow injecting interrupt before DRIVER_OK is set
[not found] <20210923075722.98-1-xieyongji@bytedance.com>
2021-09-26 2:28 ` [PATCH] vduse: Disallow injecting interrupt before DRIVER_OK is set Jason Wang
@ 2021-10-13 11:26 ` Michael S. Tsirkin
1 sibling, 0 replies; 2+ messages in thread
From: Michael S. Tsirkin @ 2021-10-13 11:26 UTC (permalink / raw)
To: Xie Yongji; +Cc: virtualization
On Thu, Sep 23, 2021 at 03:57:22PM +0800, Xie Yongji wrote:
> The interrupt callback should not be triggered before DRIVER_OK
> is set. Otherwise, it might break the virtio device driver.
> So let's add a check to avoid the unexpected behavior.
>
> Fixes: c8a6153b6c59 ("vduse: Introduce VDUSE - vDPA Device in Userspace")
> Signed-off-by: Xie Yongji <xieyongji@bytedance.com>
OK I guess, except - what about locking/ordering in all this?
> ---
> drivers/vdpa/vdpa_user/vduse_dev.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
> index 29a38ecba19e..972c13a7e5da 100644
> --- a/drivers/vdpa/vdpa_user/vduse_dev.c
> +++ b/drivers/vdpa/vdpa_user/vduse_dev.c
> @@ -968,6 +968,10 @@ static long vduse_dev_ioctl(struct file *file, unsigned int cmd,
> break;
> }
> 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);
> break;
> @@ -1047,6 +1051,10 @@ 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;
> --
> 2.11.0
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 2+ messages in thread