On Tue, Feb 3, 2026 at 11:23 AM Jason Wang <[email protected]> wrote: > > On Mon, Feb 2, 2026 at 11:13 AM Zhang Tianci > <[email protected]> wrote: > > > > 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: [email protected] > > Signed-off-by: Zhang Tianci <[email protected]> > > Reviewed-by: Xie Yongji <[email protected]> > > --- > > v2: > > - Rewrite commit message. [Michael] > > - Add Fixes tag and cc stable email list. [Eugenio] > > - Rewrite one comment. [Michael] > > > > v1: https://lkml.org/lkml/2026/1/30/323 > > > > drivers/vdpa/vdpa_user/vduse_dev.c | 30 ++++++++++++++++++++++-------- > > 1 file changed, 22 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c > > b/drivers/vdpa/vdpa_user/vduse_dev.c > > index ae357d014564c..a70d0580d54e8 100644 > > --- a/drivers/vdpa/vdpa_user/vduse_dev.c > > +++ b/drivers/vdpa/vdpa_user/vduse_dev.c > > @@ -325,6 +325,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; > > > > @@ -339,7 +340,7 @@ static ssize_t vduse_dev_read_iter(struct kiocb *iocb, > > struct iov_iter *to) > > > > ret = -EAGAIN; > > if (file->f_flags & O_NONBLOCK) > > - goto unlock; > > + break; > > > > spin_unlock(&dev->msg_lock); > > ret = wait_event_interruptible_exclusive(dev->waitq, > > @@ -349,17 +350,30 @@ static ssize_t vduse_dev_read_iter(struct kiocb > > *iocb, struct iov_iter *to) > > > > spin_lock(&dev->msg_lock); > > } > > + if (!msg) { > > + spin_unlock(&dev->msg_lock); > > + return ret; > > + } > > Nit: this check seems to be redundant, I'd suggest to > > 1) move the spin_unlock() before the check of file->f_flags & O_NONBLOCK > 2) then we can simply do "return ret" when it's a nonblocking read. > > > + > > + 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) {
Btw, I would like to explain why it's still safe if a (malicious) userspace writes in this window in either commit log or here. > > + spin_lock(&dev->msg_lock); > > + /* Roll back: move msg back to send_list if still pending. > > */ > > + msg = vduse_find_msg(&dev->recv_list, req.request_id); > > + if (msg) > > + vduse_enqueue_msg(&dev->send_list, msg); > > + spin_unlock(&dev->msg_lock); > > ret = -EFAULT; > > - vduse_enqueue_msg(&dev->send_list, msg); > > - goto unlock; > > } > > - vduse_enqueue_msg(&dev->recv_list, msg); > > -unlock: > > - spin_unlock(&dev->msg_lock); > > > > return ret; > > } > > -- > > 2.39.5 > > > > Thanks Thanks
