mirror of
https://github.com/torvalds/linux.git
synced 2026-07-27 17:47:41 +02:00
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: c8a6153b6c ("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>
This commit is contained in:
parent
373ec43ded
commit
ae9c13b6fd
|
|
@ -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;
|
||||
|
||||
ret = -EAGAIN;
|
||||
if (file->f_flags & O_NONBLOCK)
|
||||
goto unlock;
|
||||
|
||||
spin_unlock(&dev->msg_lock);
|
||||
|
||||
if (file->f_flags & O_NONBLOCK)
|
||||
return -EAGAIN;
|
||||
|
||||
ret = wait_event_interruptible_exclusive(dev->waitq,
|
||||
!list_empty(&dev->send_list));
|
||||
if (ret)
|
||||
|
|
@ -388,18 +388,35 @@ static ssize_t vduse_dev_read_iter(struct kiocb *iocb, struct iov_iter *to)
|
|||
|
||||
spin_lock(&dev->msg_lock);
|
||||
}
|
||||
spin_unlock(&dev->msg_lock);
|
||||
ret = copy_to_iter(&msg->req, size, to);
|
||||
spin_lock(&dev->msg_lock);
|
||||
if (ret != size) {
|
||||
ret = -EFAULT;
|
||||
vduse_enqueue_msg_head(&dev->send_list, msg);
|
||||
goto unlock;
|
||||
}
|
||||
|
||||
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);
|
||||
unlock:
|
||||
spin_unlock(&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;
|
||||
}
|
||||
|
||||
return ret;
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user