On 9/16/26 14:31, Stefano Garzarella wrote:
> On Tue, Sep 15, 2026 at 03:15:16PM +0200, Michal Luczaj wrote:
>> Virtio/PM events are serviced by virtio_vsock_reset_sock(), which resets
>
> What "PM" means here?
Power Management, namely virtio_vsock_driver.freeze which is defined under
CONFIG_PM_SLEEP. I'll generalize this comment, as suggested below.
>> each connected socket. The reset is done under vsock_table_lock but without
>> taking lock_sock(), so from the point of view of vsock_connect() -
>> locklessly. The same pattern exists in VMCI's
>> vmci_transport_handle_detach() and vhost's vhost_vsock_reset_orphans().
>>
>> The complexity of connect() comes from the fact that:
>> 1. the virtio transport can be reassigned, so the old transport must be
>> safely released;
>> 2. a failed connect can be followed by a retry, so the socket must be
>> reverted to a sensible state.
>> Both cases apply only as long as the socket has not yet established a
>> connection.
>>
>> While connect() waits for TCP_SYN_SENT -> TCP_ESTABLISHED, other
>> transitions can also occur:
>>
>> TCP_SYN_SENT -> TCP_CLOSE on connection failure, timeout or signal
>> TCP_SYN_SENT -> TCP_ESTABLISHED -> TCP_CLOSING on VIRTIO_VSOCK_OP_RST
>> TCP_SYN_SENT -> TCP_ESTABLISHED -> [TCP_CLOSING ->] TCP_CLOSE on event
>>
>> This further complicates connect(). Rather than making every event handler
>> drop the socket from connected_table or adapting connect() to handle more
>> transitions (while missing proper locking), use vsk->peer_shutdown as a
>> poison flag. Whatever state an event leaves the socket in, the flag bricks
>> it and prevents suspicious transport reassignments or TCP_SYN_SENT
>> retransmissions.
>>
>> Fixes: d021c344051a ("VSOCK: Introduce VM Sockets")
>> Signed-off-by: Michal Luczaj <[email protected]>
>> ---
>> net/vmw_vsock/af_vsock.c | 6 ++++++
>> 1 file changed, 6 insertions(+)
>>
>> diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
>> index adf3f018347e..972952d04a81 100644
>> --- a/net/vmw_vsock/af_vsock.c
>> +++ b/net/vmw_vsock/af_vsock.c
>> @@ -1743,6 +1743,12 @@ static int vsock_connect(struct socket *sock, struct
>> sockaddr_unsized *addr,
>> goto out;
>> }
>>
>> + /* Virtio/PM events are serviced locklessly. */
>
> IMO we should be generic here (i.e. don't mention virtio or mention it
> like one of the transport, but IIUC also VMCI does something similar)
> and also we should explain better why we are doing this, like you did in
> the commit description.
Sure, will do.
> Maybe we should document this behaviour also on top of this file.
OK, do you mind if I do that as a follow up? I worry top of the file needs
few more updates.
>> + if (READ_ONCE(vsk->peer_shutdown)) {
>> + err = -ECONNRESET;
>
> Is ECONNRESET a valid connect() error to return?
>
>> + goto out;
>> + }
>> +
>
> From LLM reviewing, can you check if it's valid? :
> - M (net/vmw_vsock/af_vsock.c:1747): VMCI regression.
> vmci_transport_handle_detach() sets
> peer_shutdown = SHUTDOWN_MASK unconditionally and then special-cases
> TCP_SYN_SENT with the
> comment "we treat the detach event like a reset" — i.e. a connect()
> retry is the expected
> recovery. It is reachable for a non-connected socket via
> vmci_transport_peer_detach_cb() (which
> uses trans->sk, not the connected table). Since vsock_assign_transport()
> only clears
> peer_shutdown when the transport actually changes (af_vsock.c:671-689),
> the retry now hits the
> new check and returns -ECONNRESET forever: the fd is permanently bricked
> where it previously
> reconnected.
I'm not sure I follow the logic. SHUTDOWN_MASK gets set no matter what's
the state at vmci_transport_handle_detach(). That means even if vmci's
reconnect succeeds, all the recvmsg()/sendmsg() at af_vsock layer would
fail anyway. Am I missing something?
thanks,
Michal
