vduse: Fix race in vduse_dev_msg_sync and vduse_dev_read_iter

There is one race case in vduse_dev_msg_sync and vduse_dev_read_iter:

vduse_dev_read_iter():
    lock(msg_lock);
    dequeue_msg(send_list);
    unlock(msg_lock);
vduse_dev_msg_sync():
    wait_timeout() finish
    lock(msg_lock);
    check msg->complete is false
        list_del(msg);   <- double list_del() crash!

To fix this case, we shall ensure vduse_msg is on send_list or recv_list
outside the msg_lock critical section.

Fixes: c8a6153b6c59 ("vduse: Introduce VDUSE - vDPA Device in Userspace")
Cc: stable@vger.kernel.org
Signed-off-by: Zhang Tianci <zhangtianci.1997@bytedance.com>
Reviewed-by: Xie Yongji <xieyongji@bytedance.com>
Acked-by: Jason Wang <jasowang@redhat.com>
Acked-by: Eugenio Pérez <eperezma@redhat.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Message-ID: <20260226115550.1814-3-zhangtianci.1997@bytedance.com>

diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
index a479fef535ac61391b8c8a9a27793f726e07e1cf..81409a5bb09bfacb2216c4ea3c4d8f4d7a965482 100644 (file)
--- a/drivers/vdpa/vdpa_user/vduse_dev.c
+++ b/drivers/vdpa/vdpa_user/vduse_dev.c
@@ -364,6 +364,7 @@ static ssize_t vduse_dev_read_iter(struct kiocb *iocb, struct iov_iter *to)
        struct file *file = iocb->ki_filp;
        struct vduse_dev *dev = file->private_data;
        struct vduse_dev_msg *msg;
+       struct vduse_dev_request req;
        int size = sizeof(struct vduse_dev_request);
        ssize_t ret;
 
@@ -375,12 +376,11 @@ static ssize_t vduse_dev_read_iter(struct kiocb *iocb, struct iov_iter *to)
                msg = vduse_dequeue_msg(&dev->send_list);
                if (msg)
                        break;
+               spin_unlock(&dev->msg_lock);
 
-               ret = -EAGAIN;
                if (file->f_flags & O_NONBLOCK)
-                       goto unlock;
+                       return -EAGAIN;
 
-               spin_unlock(&dev->msg_lock);
                ret = wait_event_interruptible_exclusive(dev->waitq,
                                        !list_empty(&dev->send_list));
                if (ret)
@@ -388,17 +388,34 @@ static ssize_t vduse_dev_read_iter(struct kiocb *iocb, struct iov_iter *to)
 
                spin_lock(&dev->msg_lock);
        }
+
+       memcpy(&req, &msg->req, sizeof(req));
+       /*
+        * We must ensure vduse_msg is on send_list or recv_list before unlock
+        * dev->msg_lock. Because vduse_dev_msg_sync() may be timeout when we
+        * copy data to userspace, and will call list_del() for this msg.
+        */
+       vduse_enqueue_msg(&dev->recv_list, msg);
        spin_unlock(&dev->msg_lock);
-       ret = copy_to_iter(&msg->req, size, to);
-       spin_lock(&dev->msg_lock);
+
+       ret = copy_to_iter(&req, size, to);
        if (ret != size) {
+               /*
+                * Roll back: move msg back to send_list if still pending.
+                *
+                * NOTE:
+                * vduse_find_msg() must use req.request_id instead of `msg`.
+                * A malicious userspace may reply to this request, and wake up
+                * the caller, after which `msg` will have already been freed.
+                * And here vduse_find_msg() will return NULL then do nothing.
+                */
+               spin_lock(&dev->msg_lock);
+               msg = vduse_find_msg(&dev->recv_list, req.request_id);
+               if (msg)
+                       vduse_enqueue_msg_head(&dev->send_list, msg);
+               spin_unlock(&dev->msg_lock);
                ret = -EFAULT;
-               vduse_enqueue_msg_head(&dev->send_list, msg);
-               goto unlock;
        }
-       vduse_enqueue_msg(&dev->recv_list, msg);
-unlock:
-       spin_unlock(&dev->msg_lock);
 
        return ret;
 }