* [PATCH v2] vsock: keep SOCK_SEQPACKET message boundaries on interrupted send
@ 2026-09-19 12:29 Bartłomiej Dmitruk
2026-09-20 9:08 ` David Laight
0 siblings, 1 reply; 2+ messages in thread
From: Bartłomiej Dmitruk @ 2026-09-19 12:29 UTC (permalink / raw)
To: Stefan Hajnoczi, Stefano Garzarella, Michael S . Tsirkin,
Jason Wang, Eugenio Pérez
Cc: Xuan Zhuo, David S . Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman, kvm, virtualization, netdev,
linux-kernel
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.
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").
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,
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH v2] vsock: keep SOCK_SEQPACKET message boundaries on interrupted send
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
0 siblings, 0 replies; 2+ messages in thread
From: David Laight @ 2026-09-20 9:08 UTC (permalink / raw)
To: Bartłomiej Dmitruk
Cc: Stefan Hajnoczi, Stefano Garzarella, Michael S . Tsirkin,
Jason Wang, Eugenio Pérez, Xuan Zhuo, David S . Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman, kvm,
virtualization, netdev, linux-kernel
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,
>
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-09-20 9:08 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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 is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®