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 <[email protected]> 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/[email protected]?part=1
