From: David Laight <david.laight.linux@gmail.com>
To: "Bartłomiej Dmitruk" <bartlomiej.dmitruk@isec.pl>
Cc: "Stefan Hajnoczi" <stefanha@redhat.com>,
"Stefano Garzarella" <sgarzare@redhat.com>,
"Michael S . Tsirkin" <mst@redhat.com>,
"Jason Wang" <jasowangio@gmail.com>,
"Eugenio Pérez" <eperezma@redhat.com>,
"Xuan Zhuo" <xuanzhuo@linux.alibaba.com>,
"David S . Miller" <davem@davemloft.net>,
"Eric Dumazet" <edumazet@google.com>,
"Jakub Kicinski" <kuba@kernel.org>,
"Paolo Abeni" <pabeni@redhat.com>,
"Simon Horman" <horms@kernel.org>,
kvm@vger.kernel.org, virtualization@lists.linux.dev,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] vsock: keep SOCK_SEQPACKET message boundaries on interrupted send
Date: Sun, 20 Sep 2026 10:08:52 +0100 [thread overview]
Message-ID: <20260920100852.7806895b@pumpkin> (raw)
In-Reply-To: <20260919122916.28226-1-bartlomiej.dmitruk@isec.pl>
On Sat, 19 Sep 2026 14:29:15 +0200
Bartłomiej Dmitruk <bartlomiej.dmitruk@isec.pl> wrote:
> 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 -- notably the non-terminal -EINTR path
> (signal_pending while blocked for credit), but also sk_err / peer
> RCV_SHUTDOWN -- 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.
Should the unsent fragments just get requeued with an EOM flag?
The receiving system will have a partial message (and should be allowed to
pass that to an application) and will append the fragments to it when
they arrive.
>
> Wait until the whole remaining SEQPACKET message fits before enqueuing, so a
> message is committed atomically (with its EOM) or not started; an error while
> waiting then leaves nothing on the wire. SOCK_STREAM behaviour is unchanged
> (min_space == 1 reproduces the old "wait while space == 0").
Why should the message ever fit?
ISO transport (which no one really uses any more) maps to SEQPACKET.
It is perfectly valid for a file transfer program to send the entire file
as one 'message' by not sending EOM until the end of file fragment.
David
>
> To avoid blocking forever on a message that can never fit -- a peer can
> advertise a small buf_alloc, or the message can simply exceed the transmit
> buffer -- reject such a message up front with -EMSGSIZE (the same error the
> transport already returns for an oversized message) instead of waiting. This
> needs the transport's maximum message size, added as a new optional
> seqpacket_max_size() transport op implemented by the virtio/loopback
> transports.
>
> Signed-off-by: Bartłomiej Dmitruk <bartlomiej.dmitruk@isec.pl>
> Assisted-by: Claude (Anthropic)
> ---
> Changes in v2:
> - Fix an indefinite wait the v1 approach introduced for oversized
> SOCK_SEQPACKET messages (reported by the Sashiko AI review on v1): the
> atomic-wait could never be satisfied when len > the transport's max
> message size (e.g. a peer advertising a small peer_buf_alloc), so the
> sender blocked instead of returning -EMSGSIZE. v2 checks this up front via
> the new seqpacket_max_size() op and returns -EMSGSIZE without waiting.
> - v1: https://lore.kernel.org/netdev/20260917220101.55744-1-bartlomiej.dmitruk@isec.pl/
>
> Testing (vsock_loopback, unprivileged):
> - Interrupted partial send (-EINTR) no longer leaks orphan bytes into the
> next message (recv() returns only the next message).
> - Oversized message and small-peer_buf_alloc cases return -EMSGSIZE promptly
> instead of blocking.
> - Normal SEQPACKET send/recv unaffected.
>
> diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h
> index f91704731..e96790c03 100644
> --- a/include/linux/virtio_vsock.h
> +++ b/include/linux/virtio_vsock.h
> @@ -219,6 +219,7 @@ virtio_transport_seqpacket_dequeue(struct vsock_sock *vsk,
> s64 virtio_transport_stream_has_data(struct vsock_sock *vsk);
> s64 virtio_transport_stream_has_space(struct vsock_sock *vsk);
> u32 virtio_transport_seqpacket_has_data(struct vsock_sock *vsk);
> +u32 virtio_transport_seqpacket_max_size(struct vsock_sock *vsk);
>
> ssize_t virtio_transport_unsent_bytes(struct vsock_sock *vsk);
>
> diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h
> index 5549298c1..d94c613ef 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);
>
> /* 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 f840498b5..ac64cb957 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) {
> + /* A SEQPACKET message must be delivered atomically, so
> + * wait until the whole remaining message fits before
> + * enqueuing. Otherwise a credit-limited partial send that
> + * later errors out (e.g. -EINTR) leaves EOM-less fragments
> + * that the peer merges into the next message.
> + *
> + * Reject a message that can never fit up front so the wait
> + * below cannot block forever (a peer may advertise a small
> + * buf_alloc); this mirrors the -EMSGSIZE the transport
> + * returns for an oversized message.
> + */
> + 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 &&
> sk->sk_err == 0 &&
> !(sk->sk_shutdown & SEND_SHUTDOWN) &&
> !(vsk->peer_shutdown & RCV_SHUTDOWN)) {
> diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c
> index 4f9aa9c4c..b7d587a20 100644
> --- a/net/vmw_vsock/virtio_transport.c
> +++ b/net/vmw_vsock/virtio_transport.c
> @@ -585,6 +585,7 @@ static struct virtio_transport virtio_transport = {
> .seqpacket_enqueue = virtio_transport_seqpacket_enqueue,
> .seqpacket_allow = virtio_transport_seqpacket_allow,
> .seqpacket_has_data = virtio_transport_seqpacket_has_data,
> + .seqpacket_max_size = virtio_transport_seqpacket_max_size,
>
> .msgzerocopy_allow = virtio_transport_msgzerocopy_allow,
>
> diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
> index f225f53ed..e10e7b958 100644
> --- a/net/vmw_vsock/virtio_transport_common.c
> +++ b/net/vmw_vsock/virtio_transport_common.c
> @@ -994,6 +994,19 @@ virtio_transport_seqpacket_enqueue(struct vsock_sock *vsk,
> }
> EXPORT_SYMBOL_GPL(virtio_transport_seqpacket_enqueue);
>
> +u32 virtio_transport_seqpacket_max_size(struct vsock_sock *vsk)
> +{
> + struct virtio_vsock_sock *vvs = vsk->trans;
> + u32 max_size;
> +
> + spin_lock_bh(&vvs->tx_lock);
> + max_size = virtio_transport_tx_buf_size(vvs);
> + spin_unlock_bh(&vvs->tx_lock);
> +
> + return max_size;
> +}
> +EXPORT_SYMBOL_GPL(virtio_transport_seqpacket_max_size);
> +
> int
> virtio_transport_dgram_dequeue(struct vsock_sock *vsk,
> struct msghdr *msg,
> diff --git a/net/vmw_vsock/vsock_loopback.c b/net/vmw_vsock/vsock_loopback.c
> index 8068d1b6e..b9a0cc861 100644
> --- a/net/vmw_vsock/vsock_loopback.c
> +++ b/net/vmw_vsock/vsock_loopback.c
> @@ -90,6 +90,7 @@ static struct virtio_transport loopback_transport = {
> .seqpacket_enqueue = virtio_transport_seqpacket_enqueue,
> .seqpacket_allow = vsock_loopback_seqpacket_allow,
> .seqpacket_has_data = virtio_transport_seqpacket_has_data,
> + .seqpacket_max_size = virtio_transport_seqpacket_max_size,
>
> .msgzerocopy_allow = vsock_loopback_msgzerocopy_allow,
>
>
next prev parent reply other threads:[~2026-09-20 9:08 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-19 12:29 [PATCH v2] vsock: keep SOCK_SEQPACKET message boundaries on interrupted send Bartłomiej Dmitruk
2026-09-20 9:08 ` David Laight [this message]
2026-09-20 12:31 ` sashiko-bot
2026-09-23 12:46 ` netdev-bot+sashiko
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260920100852.7806895b@pumpkin \
--to=david.laight.linux@gmail.com \
--cc=bartlomiej.dmitruk@isec.pl \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=eperezma@redhat.com \
--cc=horms@kernel.org \
--cc=jasowangio@gmail.com \
--cc=kuba@kernel.org \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mst@redhat.com \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=sgarzare@redhat.com \
--cc=stefanha@redhat.com \
--cc=virtualization@lists.linux.dev \
--cc=xuanzhuo@linux.alibaba.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.