Re: [PATCH net v2 2/2] vsock/virtio: restore msg_iter on transmission failure

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Use of `iov_iter_restore` causes spurious `WARN_ON_ONCE` for `ITER_KVEC` iterators and lacks support for `ITER_FOLIOQ`, risking state corruption.
--

commit adc9013e2e0f53e4c04e792c4d6b248a3f5afb54
Author: Octavian Purdila <tavip@google.com>

vsock/virtio: restore msg_iter on transmission failure

This commit addresses an iterator desync issue when packet allocation or
transmission fails by saving the msg_iter state before attempting to allocate
and send, and restoring it if an error occurs.

> --- a/net/vmw_vsock/virtio_transport_common.c
> +++ b/net/vmw_vsock/virtio_transport_common.c
> @@ -368,8 +369,17 @@ static int virtio_transport_send_pkt_info(struct vsock_sock *vsk,
>  		struct sk_buff *skb;
>  		size_t skb_len;
>  
> +		/* Save iterator state in case allocation or transmission fails
> +		 * so we can restore it and retry.
> +		 */
> +		if (info->msg)
> +			iov_iter_save_state(&info->msg->msg_iter, &msg_iter_state);
> +
>  		skb_len = min(max_skb_len, rest_len);
>  
> +		/* Note: virtio_transport_alloc_skb() can advance info->msg->msg_iter
> +		 * even if it fails (e.g. partial GUP success).
> +		 */
>  		skb = virtio_transport_alloc_skb(info, skb_len, can_zcopy,
>  						 uarg,
>  						 src_cid, src_port,

[ ... ]

> @@ -399,6 +409,9 @@ static int virtio_transport_send_pkt_info(struct vsock_sock *vsk,
>  			break;
>  	} while (rest_len);
>  
> +	if (info->msg && ret < 0)
> +		iov_iter_restore(&info->msg->msg_iter, &msg_iter_state);
> +

[Severity: Medium]
Does this iov_iter_restore() call risk triggering a spurious warning for
ITER_KVEC iterators or silently failing for ITER_FOLIOQ?

In-kernel socket consumers, such as the Network Block Device, actively
construct ITER_KVEC iterators for request headers and send them via
sock_sendmsg(). Users can configure NBD over a vsock socket.

If memory pressure causes virtio_transport_alloc_skb() to fail during an
NBD transmission, the error path invokes iov_iter_restore() which evaluates
the following condition:

lib/iov_iter.c:iov_iter_restore() {
	if (WARN_ON_ONCE(!iov_iter_is_bvec(i) && !iter_is_iovec(i) &&
			 !iter_is_ubuf(i)) && !iov_iter_is_kvec(i))
		return;
...
}

For ITER_KVEC iterators, the first part of the expression evaluates to true,
unconditionally triggering a kernel warning before restoring the iterator. On
systems with panic_on_warn enabled, this could lead to a kernel panic.

Additionally, iov_iter_restore() explicitly ignores ITER_FOLIOQ iterators,
meaning it would silently fail to restore the state if the VFS passes such
an iterator, potentially leading to infinite loops and desynchronization.

Could tracking the number of bytes advanced and using iov_iter_revert()
provide a safer way to handle this rollback?

>  	virtio_transport_put_credit(vvs, rest_len);
>  
>  	/* msg_zerocopy_realloc() initializes the ubuf_info refcnt to 1.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260613000953.467473-1-tavip@google.com?part=2