drivers/vdpa/vdpa_user/vduse_dev.c | 30 ++++++++++++++++++++++-------- 1 file changed, 22 insertions(+), 8 deletions(-)
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>
---
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;
+ }
+
+ 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) {
+ 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
On Mon, Feb 2, 2026 at 11:13 AM Zhang Tianci
<zhangtianci.1997@bytedance.com> 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: stable@vger.kernel.org
> Signed-off-by: Zhang Tianci <zhangtianci.1997@bytedance.com>
> Reviewed-by: Xie Yongji <xieyongji@bytedance.com>
> ---
> 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) {
> + 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
Hi Jason,
On Tue, Feb 3, 2026 at 11:23 AM Jason Wang <jasowang@redhat.com> wrote:
>
> On Mon, Feb 2, 2026 at 11:13 AM Zhang Tianci
> <zhangtianci.1997@bytedance.com> 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: stable@vger.kernel.org
> > Signed-off-by: Zhang Tianci <zhangtianci.1997@bytedance.com>
> > Reviewed-by: Xie Yongji <xieyongji@bytedance.com>
> > ---
> > 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.
Makes sense. I'll make the change.
>
> > +
> > + 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) {
> > + 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,
Tianci
On Tue, Feb 3, 2026 at 11:23 AM Jason Wang <jasowang@redhat.com> wrote:
>
> On Mon, Feb 2, 2026 at 11:13 AM Zhang Tianci
> <zhangtianci.1997@bytedance.com> 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: stable@vger.kernel.org
> > Signed-off-by: Zhang Tianci <zhangtianci.1997@bytedance.com>
> > Reviewed-by: Xie Yongji <xieyongji@bytedance.com>
> > ---
> > 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
Hi, Jason,
On Tue, Feb 3, 2026 at 11:27 AM Jason Wang <jasowang@redhat.com> wrote:
>
> On Tue, Feb 3, 2026 at 11:23 AM Jason Wang <jasowang@redhat.com> wrote:
> >
> > On Mon, Feb 2, 2026 at 11:13 AM Zhang Tianci
> > <zhangtianci.1997@bytedance.com> 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: stable@vger.kernel.org
> > > Signed-off-by: Zhang Tianci <zhangtianci.1997@bytedance.com>
> > > Reviewed-by: Xie Yongji <xieyongji@bytedance.com>
> > > ---
> > > 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.
Do you mean we should document in a comment here why the potential read/write
race is safe?
Thanks,
Tianci
© 2016 - 2026 Red Hat, Inc.