Re: [PATCH v2] vsock: keep SOCK_SEQPACKET message boundaries on interrupted send

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] vsock: vhost_transport missing seqpacket_max_size causes indefinite hang
- [Medium] vsock: seqpacket wait loop hangs if peer dynamically shrinks buffer
--

commit faa45a951be47102c344158967938f9970eb6270
Author: Bartłomiej Dmitruk <bartlomiej.dmitruk@isec.pl>

vsock: keep SOCK_SEQPACKET message boundaries on interrupted send

A credit-limited SOCK_SEQPACKET send transmits fragments as credit becomes
available, and the VIRTIO_VSOCK_SEQ_EOM flag is set only on the fragment
where msg_data_left() reaches 0. If vsock_connectible_sendmsg() exits via
out_err after a partial send, the already-transmitted fragments carry no EOM.
The receiver only advances msg_count / sets msg_ready on an EOM skb, so the
orphaned fragments are silently merged into the next message, violating
SOCK_SEQPACKET atomicity.

> diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h
> index 5549298c1ec6b..d94c613ef604b 100644
> --- a/include/net/af_vsock.h
> +++ b/include/net/af_vsock.h
> @@ -143,6 +143,7 @@ struct vsock_transport {
>  				 size_t len);
>  	bool (*seqpacket_allow)(struct vsock_sock *vsk, u32 remote_cid);
>  	u32 (*seqpacket_has_data)(struct vsock_sock *vsk);
> +	u32 (*seqpacket_max_size)(struct vsock_sock *vsk);
>  

[Severity: High]
Does vhost_transport in drivers/vhost/vsock.c also need to implement this
callback?

It implements SOCK_SEQPACKET, but appears to omit the newly required
seqpacket_max_size() callback. If the callback is NULL, the max size check in
vsock_connectible_sendmsg() is skipped. If a user application sends a
SOCK_SEQPACKET message larger than the transport's buffer size, this will
cause an indefinite wait loop because the space can never reach min_space.

>  	/* Notification. */
>  	int (*notify_poll_in)(struct vsock_sock *, size_t, bool *);

[ ... ]

> diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
> index 9b71479a2b295..01fc9ac6fa2b5 100644
> --- a/net/vmw_vsock/af_vsock.c
> +++ b/net/vmw_vsock/af_vsock.c
> @@ -2250,9 +2250,32 @@ static int vsock_connectible_sendmsg(struct socket *sock, struct msghdr *msg,
>  
>  	while (total_written < len) {
>  		ssize_t written;
> +		s64 min_space;
> +
> +		if (sk->sk_type == SOCK_SEQPACKET) {

[ ... ]

> +			if (transport->seqpacket_max_size &&
> +			    len > transport->seqpacket_max_size(vsk)) {
> +				err = -EMSGSIZE;
> +				goto out_err;
> +			}
> +			min_space = len - total_written;
> +		} else {
> +			min_space = 1;
> +		}
>  
>  		add_wait_queue(sk_sleep(sk), &wait);
> -		while (vsock_stream_has_space(vsk) == 0 &&
> +		while (vsock_stream_has_space(vsk) < min_space &&

[Severity: Medium]
What happens if the remote peer dynamically shrinks its receive buffer to a
size smaller than min_space while the sender is waiting in this loop?

Since the seqpacket_max_size() check is outside the sleep loop, if the space
ceiling shrinks below min_space during the wait, the sender could be
permanently trapped without re-evaluating the max size.

>  		       sk->sk_err == 0 &&

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260919122916.28226-1-bartlomiej.dmitruk@isec.pl?part=1