On Thu, Aug 13, 2026 at 11:42:17AM +0200, Michal Luczaj wrote:
> On 8/12/26 22:13, Hyunwoo Kim wrote:
> > commit 002541ef650b ("vsock: Ignore signal/timeout on connect() if
> > already established") stopped connect() from resetting an established
> > socket. The check only looks at whether sk_state is TCP_ESTABLISHED at
> > that moment, and the state can change while connect() sleeps.
> 
> I guess this makes my fix incomplete. "Fixes: 002541ef650b"?

It is incomplete, yes. But this has been triggerable since d021c344051a, so
I'd keep Fixes: d021c344051a.

> 
> > A peer RST moves the socket to TCP_CLOSING, and it is not removed from
> > vsock_connected_table on that path. connect() then wakes up, fails the
> > check, and resets a socket that had actually connected to TCP_CLOSE and
> > SS_UNCONNECTED.
> 
> Thanks for the details. Do I get it right: connect() misses the fact that
> socket might have already transitioned TCP_ESTABLISHED -> TCP_CLOSING
> during schedule_timeout()?

Yes, that's it.

> 
> How about:
> 
> diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
> index 622dbd046799..39c42ef016c3 100644
> --- a/net/vmw_vsock/af_vsock.c
> +++ b/net/vmw_vsock/af_vsock.c
> @@ -1807,15 +1807,18 @@ static int vsock_connect(struct socket *sock, struct 
> sockaddr_unsized *addr,
>               timeout = schedule_timeout(timeout);
>               lock_sock(sk);
>  
> -             /* Connection established. Whatever happens to socket once we
> +             /* Connection (has been) established. Whatever happens to 
> socket once we
>                * release it, that's not connect()'s concern. No need to go
>                * into signal and timeout handling. Call it a day.
>                *
>                * Note that allowing to "reset" an already established socket
>                * here is racy and insecure.
>                */
> -             if (sk->sk_state == TCP_ESTABLISHED)
> -                     break;
> +             if (sk->sk_state == TCP_ESTABLISHED ||
> +                 sk->sk_state == TCP_CLOSING) {
> +                     err = -sk->sk_err;
> +                     goto out_wait;
> +             }
>  
>               /* If connection was _not_ established and a signal/timeout came
>                * to be, we want the socket's state reset. User space may want
> 
> ?

Yes, I like it better than mine. I confirmed it fixes the issue.

If you don't mind, would you take the patch from here?


Best regards,
Hyunwoo Kim

Reply via email to