Re: [RFC PATCH] vhost: coalesce and retry IOTLB miss messages

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Guest can intentionally trigger a `WARN_ON_ONCE` in `vhost_iotlb_miss`, causing a host kernel panic if `panic_on_warn` is enabled.
- [High] In-flight IOTLB miss node escapes `vhost_clear_msg` during backend detach, causing a permanent virtqueue stall.
- [High] Virtqueues on `iotlb_wait_list` are ignored by `IOTLB_UPDATE` notifications, resulting in permanent deadlocks.
--

--- Patch [1]: [RFC PATCH] vhost: coalesce and retry IOTLB miss messages ---
commit df21dbd6b3d7b91189f87885be5157a564b6e59e
Author: Jia Jia <physicalmtea@gmail.com>

vhost: coalesce and retry IOTLB miss messages

This commit introduces a mechanism to coalesce and retry IOTLB miss
messages in the vhost subsystem. It limits the total number of in-flight
IOTLB messages device-wide and keeps a per-virtqueue record of the current
miss to prevent duplicate allocations.

> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index a0c1d54019aac..d5de41fdc3c05 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c

[ ... ]

> @@ -1191,20 +1198,104 @@ void vhost_dev_stop(struct vhost_dev *dev)
>  }
>  EXPORT_SYMBOL_GPL(vhost_dev_stop);
>  

[ ... ]

>  void vhost_clear_msg(struct vhost_dev *dev)
>  {
>  	struct vhost_msg_node *node, *n;
> +	struct vhost_virtqueue *vq, *vq_n;
> +	int i;
>  
>  	spin_lock(&dev->iotlb_lock);
>  
>  	list_for_each_entry_safe(node, n, &dev->read_list, node) {
> -		list_del(&node->node);
> -		kfree(node);
> +		list_del_init(&node->node);
> +		__vhost_free_msg(dev, node);
>  	}
>  
>  	list_for_each_entry_safe(node, n, &dev->pending_list, node) {
> -		list_del(&node->node);
> -		kfree(node);
> +		list_del_init(&node->node);
> +		__vhost_free_msg(dev, node);
> +	}
> +
> +	if (!dev->iotlb) {
> +		list_for_each_entry_safe(vq, vq_n, &dev->iotlb_wait_list,
> +					 iotlb_miss_wait_node) {
> +			list_del_init(&vq->iotlb_miss_wait_node);
> +			vq->iotlb_miss_waiting = false;
> +		}
> +
> +		for (i = 0; i < dev->nvqs; ++i)
> +			dev->vqs[i]->iotlb_miss_node = NULL;
> +	}

[Severity: High]
Can an in-flight IOTLB miss node escape being cleared here during backend
detach, resulting in a permanently stalled virtqueue?

If a userspace orchestrator detaches the backend while a thread is reading an
IOTLB miss via vhost_chr_read_iter(), the node is temporarily dequeued for
copy_to_iter(). This function skips the node since it is not on any list.

Because dev->iotlb is still active during backend detach, this fallback
cleanup is skipped and vq->iotlb_miss_node is not cleared. When the node is
later enqueued to the pending_list and a new backend is attached, the next
IOTLB miss will hit the WARN_ON_ONCE(vq->iotlb_miss_node) check and abort
indefinitely.

>  	spin_unlock(&dev->iotlb_lock);

[ ... ]

> @@ -1602,18 +1693,25 @@ static inline int vhost_get_desc(struct vhost_virtqueue *vq,
>  static void vhost_iotlb_notify_vq(struct vhost_dev *d,
>  				  struct vhost_iotlb_msg *msg)
>  {
> -	struct vhost_msg_node *node, *n;
> +	int i;
>  
>  	spin_lock(&d->iotlb_lock);
>  
> -	list_for_each_entry_safe(node, n, &d->pending_list, node) {
> -		struct vhost_iotlb_msg *vq_msg = &node->msg.iotlb;
> -		if (msg->iova <= vq_msg->iova &&
> -		    msg->iova + msg->size - 1 >= vq_msg->iova &&
> -		    vq_msg->type == VHOST_IOTLB_MISS) {
> -			vhost_poll_queue(&node->vq->poll);
> -			list_del(&node->node);
> -			kfree(node);
> +	for (i = 0; i < d->nvqs; ++i) {
> +		struct vhost_virtqueue *vq = d->vqs[i];
> +		struct vhost_msg_node *node = vq->iotlb_miss_node;
> +
> +		if (!node || msg->iova > node->iotlb_miss_iova ||
> +		    msg->iova + msg->size - 1 < node->iotlb_miss_iova)
> +			continue;

[Severity: High]
Does this loop skip virtqueues that are on the iotlb_wait_list?

When the 2048 message limit is reached, vhost_iotlb_miss() places the
virtqueue on the wait list and returns -ENOSPC without allocating a node,
leaving vq->iotlb_miss_node as NULL.

When userspace later sends an IOTLB_UPDATE for the requested mapping, this
check ignores virtqueues with !node. This means the waiting virtqueues are
never woken up to use the new mapping and stall permanently.

> +		vq->iotlb_miss_node = NULL;

[ ... ]

> @@ -1858,8 +1986,40 @@ static int vhost_iotlb_miss(struct vhost_virtqueue *vq, u64 iova, int access)

[ ... ]

>  	msg->type = VHOST_IOTLB_MISS;
>  	msg->iova = iova;
>  	msg->perm = access;
> +	node->iotlb_miss = true;
> +	node->iotlb_miss_iova = iova;
> +	node->iotlb_miss_perm = access;
>  
> -	vhost_enqueue_msg(dev, &dev->read_list, node);
> +	spin_lock(&dev->iotlb_lock);
> +	if (vhost_iotlb_miss_exists(vq, iova, access)) {
> +		vhost_unwait_iotlb_miss(dev, vq);
> +		goto unlock;
> +	}
> +	if (WARN_ON_ONCE(vq->iotlb_miss_node)) {
> +		ret = -EAGAIN;
> +		goto unlock;
> +	}

[Severity: High]
Is it possible for an unprivileged guest to intentionally trigger this
WARN_ON_ONCE?

If a guest triggers an IOTLB miss, vhost allocates a miss node. Before
userspace resolves the miss, the guest could overwrite the descriptor's IOVA in
shared memory and issue another kick.

The vhost worker would attempt translation again, fail for the new IOVA, and
re-enter vhost_iotlb_miss(). Because the IOVA changed,
vhost_iotlb_miss_exists() returns false, leading directly to this
WARN_ON_ONCE. This would cause a host kernel panic if panic_on_warn is
enabled.

> +	if (dev->iotlb_msg_count >= VHOST_MAX_IOTLB_MSGS) {
> +		vhost_wait_iotlb_miss(dev, vq);
> +		ret = -ENOSPC;
> +		goto unlock;
> +	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260806140616.59025-1-physicalmtea@gmail.com?part=1