On Thu, Jul 23, 2026 at 06:33:10PM +0300, Andrey Drobyshev wrote:
Every other update of vq->worker is done under vq->mutex - the worker
attach/swap ioctls and vhost_worker_killed(). vhost_workers_free() is
the sole exception: it clears vq->worker without holding the lock.
mmm, vhost_dev_cleanup() updates vq->worker without the mutex too IIUC.
The effect is harmless in practice, as this only happens while the
owning process (and thus the whole device) is dying, but the lockless
write is inconsistent with the rest of the code. Clear vq->worker under
vq->mutex, like everyone else, so that all writers of vq->worker follow
the same locking rule.
This issue was found by Sashiko AI review.
Can you share a link to the review?
I don't know if it's common or not, but having the link in the commit or
after --- will help the reviewers.
Signed-off-by: Andrey Drobyshev <[email protected]>
---
drivers/vhost/vhost.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 4c525b3e16ea..dbb6cb5eccea 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -722,13 +722,19 @@ static void vhost_worker_destroy(struct vhost_dev *dev,
static void vhost_workers_free(struct vhost_dev *dev)
{
struct vhost_worker *worker;
+ struct vhost_virtqueue *vq;
unsigned long i;
if (!dev->use_worker)
return;
- for (i = 0; i < dev->nvqs; i++)
- rcu_assign_pointer(dev->vqs[i]->worker, NULL);
+ for (i = 0; i < dev->nvqs; i++) {
+ vq = dev->vqs[i];
+
+ mutex_lock(&vq->mutex);
+ rcu_assign_pointer(vq->worker, NULL);
+ mutex_unlock(&vq->mutex);
+ }
Pre-existing, but IIUC vhost_workers_free() is called only by
vhost_dev_cleanup() at the bottom, after a loop calls vhost_vq_reset()
on each virtqueue (without the mutex) where we already set `vq->worker`
to NULL, so IMO at this point it's already NULL, no?
Thanks,
Stefano
/*
* Free the default worker we created and cleanup workers userspace
* created but couldn't clean up (it forgot or crashed).
--
2.47.1
