[PATCH net] vsock: do not reset a socket in connect() once it has connected

[PATCH net] vsock: do not reset a socket in connect() once it has connected

[Hyunwoo Kim]

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.

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.

The socket can now be connected again while it is still on the table.
Reconnecting to an address served by a different transport makes
vsock_assign_transport() drop the transport from a live socket and free
vsk->trans, even if skbs it already sent are still in flight.
Reconnecting to the same address inserts a node that is already on the
table, provided shutdown() has cleared SOCK_DONE in between.

sock->state cannot be used for the check either. shutdown() overwrites
SS_CONNECTED with SS_DISCONNECTING.

Record on the socket that the connection completed, and check that
instead. The sk_err path after the loop does the same reset, so guard it
as well.

Fixes: d021c344051a ("VSOCK: Introduce VM Sockets")
Cc: stable@vger.kernel.org
Signed-off-by: Hyunwoo Kim <imv4bel@gmail.com>
---
 include/net/af_vsock.h   |  2 ++
 net/vmw_vsock/af_vsock.c | 12 ++++++++----
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h
index 30046a3c20f735..d7ec976ed6e272 100644
--- a/include/net/af_vsock.h
+++ b/include/net/af_vsock.h
@@ -63,6 +63,8 @@ struct vsock_sock {
 	struct delayed_work pending_work;
 	struct delayed_work close_work;
 	bool close_work_scheduled;
+	/* Set once the connection completed; never cleared. */
+	bool ever_connected;
 	u32 peer_shutdown;
 	bool sent_request;
 	bool ignore_connecting_rst;
diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
index 622dbd04679944..735c91bb762242 100644
--- a/net/vmw_vsock/af_vsock.c
+++ b/net/vmw_vsock/af_vsock.c
@@ -351,6 +351,8 @@ void vsock_insert_connected(struct vsock_sock *vsk)
 	struct list_head *list = vsock_connected_sockets(
 		&vsk->remote_addr, &vsk->local_addr);
 
+	vsk->ever_connected = true;
+
 	spin_lock_bh(&vsock_table_lock);
 	__vsock_insert_connected(list, vsk);
 	spin_unlock_bh(&vsock_table_lock);
@@ -1814,14 +1816,14 @@ static int vsock_connect(struct socket *sock, struct sockaddr_unsized *addr,
 		 * Note that allowing to "reset" an already established socket
 		 * here is racy and insecure.
 		 */
-		if (sk->sk_state == TCP_ESTABLISHED)
+		if (vsk->ever_connected)
 			break;
 
 		/* If connection was _not_ established and a signal/timeout came
 		 * to be, we want the socket's state reset. User space may want
 		 * to retry.
 		 *
-		 * sk_state != TCP_ESTABLISHED implies that socket is not on
+		 * !ever_connected implies that socket is not on
 		 * vsock_connected_table. We keep the binding and the transport
 		 * assigned.
 		 */
@@ -1849,8 +1851,10 @@ static int vsock_connect(struct socket *sock, struct sockaddr_unsized *addr,
 
 	if (sk->sk_err) {
 		err = -sk->sk_err;
-		sk->sk_state = TCP_CLOSE;
-		sock->state = SS_UNCONNECTED;
+		if (!vsk->ever_connected) {
+			sk->sk_state = TCP_CLOSE;
+			sock->state = SS_UNCONNECTED;
+		}
 	} else {
 		err = 0;
 	}
-- 
2.43.0

Re: [PATCH net] vsock: do not reset a socket in connect() once it has connected

[Michal Luczaj]

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"?

> 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()?

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

?

> The socket can now be connected again while it is still on the table.
> Reconnecting to an address served by a different transport makes
> vsock_assign_transport() drop the transport from a live socket and free
> vsk->trans, even if skbs it already sent are still in flight.
> Reconnecting to the same address inserts a node that is already on the
> table, provided shutdown() has cleared SOCK_DONE in between.
> 
> sock->state cannot be used for the check either. shutdown() overwrites
> SS_CONNECTED with SS_DISCONNECTING.
> 
> Record on the socket that the connection completed, and check that
> instead. The sk_err path after the loop does the same reset, so guard it
> as well.

Re: [PATCH net] vsock: do not reset a socket in connect() once it has connected

[Hyunwoo Kim]

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

Re: [PATCH net] vsock: do not reset a socket in connect() once it has connected

[Michal Luczaj]

On 8/14/26 03:45, Hyunwoo Kim wrote:
> 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.

OK, I get it.

>>> 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.

Great, thanks.

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

Sure, no problem.

Stefano, does this look good to you? And should any sk_err be consumed
here, too? (`err = sock_error(sk)` instead of `err = -sk->sk_err`)

thanks,
Michal

Re: [PATCH net] vsock: do not reset a socket in connect() once it has connected

[Stefano Garzarella]

On Mon, Aug 17, 2026 at 12:29:11AM +0200, Michal Luczaj wrote:
>On 8/14/26 03:45, Hyunwoo Kim wrote:
>> 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.
>
>OK, I get it.
>
>>>> 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.
>
>Great, thanks.
>
>> If you don't mind, would you take the patch from here?
>
>Sure, no problem.
>
>Stefano, does this look good to you?

Yep, thanks for helping here!

My only doubt is if it makes sense to leave the `break` there, and add a 
similar check before resetting the socket, I mean something like this:

	err = sock_error(sk);
	if (err && sk->sk_state != TCP_ESTABLISHED &&
		   sk->sk_state != TCP_CLOSING)) {
		sk->sk_state = TCP_CLOSE;
		sock->state = SS_UNCONNECTED;
	}

Just to be a bit more defensive, but I don't have a strong opinion, your 
version is also fine.

>And should any sk_err be consumed
>here, too? (`err = sock_error(sk)` instead of `err = -sk->sk_err`)

I'd stay with sock_error() to consume the error if it makes sense also 
for you.

Thanks,
Stefano

Re: [PATCH net] vsock: do not reset a socket in connect() once it has connected

[Hyunwoo Kim]

On Wed, Aug 19, 2026 at 10:54:38AM +0200, Stefano Garzarella wrote:
> On Mon, Aug 17, 2026 at 12:29:11AM +0200, Michal Luczaj wrote:
> > On 8/14/26 03:45, Hyunwoo Kim wrote:
> > > 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.
> > 
> > OK, I get it.
> > 
> > > > > 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.
> > 
> > Great, thanks.
> > 
> > > If you don't mind, would you take the patch from here?
> > 
> > Sure, no problem.
> > 
> > Stefano, does this look good to you?
> 
> Yep, thanks for helping here!
> 
> My only doubt is if it makes sense to leave the `break` there, and add a
> similar check before resetting the socket, I mean something like this:
> 
> 	err = sock_error(sk);
> 	if (err && sk->sk_state != TCP_ESTABLISHED &&
> 		   sk->sk_state != TCP_CLOSING)) {
> 		sk->sk_state = TCP_CLOSE;
> 		sock->state = SS_UNCONNECTED;
> 	}
> 
> Just to be a bit more defensive, but I don't have a strong opinion, your
> version is also fine.
> 
> > And should any sk_err be consumed
> > here, too? (`err = sock_error(sk)` instead of `err = -sk->sk_err`)
> 
> I'd stay with sock_error() to consume the error if it makes sense also for
> you.
> 
> Thanks,
> Stefano
> 

Gentle ping. I'd appreciate an update on where this stands.


Best regards,
Hyunwoo Kim

Re: [PATCH net] vsock: do not reset a socket in connect() once it has connected

[Michal Luczaj]

On 9/7/26 20:35, Hyunwoo Kim wrote:
> Gentle ping. I'd appreciate an update on where this stands.
Thanks for poking me. I'll post a patch today or tomorrow.

Apologies for the delay,
Michal

Re: [PATCH net] vsock: do not reset a socket in connect() once it has connected

[Michal Luczaj]

On 8/19/26 10:54, Stefano Garzarella wrote:
>>>> 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.
>>
>> Great, thanks.
>>
>>> If you don't mind, would you take the patch from here?
>>
>> Sure, no problem.
>>
>> Stefano, does this look good to you?
> 
> Yep, thanks for helping here!
> 
> My only doubt is if it makes sense to leave the `break` there, and add a 
> similar check before resetting the socket, I mean something like this:
> 
> 	err = sock_error(sk);
> 	if (err && sk->sk_state != TCP_ESTABLISHED &&
> 		   sk->sk_state != TCP_CLOSING)) {
> 		sk->sk_state = TCP_CLOSE;
> 		sock->state = SS_UNCONNECTED;
> 	}
> 
> Just to be a bit more defensive, but I don't have a strong opinion, your 
> version is also fine.
> 
>> And should any sk_err be consumed
>> here, too? (`err = sock_error(sk)` instead of `err = -sk->sk_err`)
> 
> I'd stay with sock_error() to consume the error if it makes sense also 
> for you.

Sure, one note: perhaps it's better to always return 0 on
TCP_ESTABLISHED/TCP_CLOSING? From connect()'s PoV these two states mean "we
did good, we connected to something for some time". Currently, even if the
connection was established, virtio_transport_recv_connected()'s
VIRTIO_VSOCK_OP_RW error handling may set sk_err = ENOBUFS, and connect()
would return that. Even though connect() itself was a success.

Anyway, here's a fix for the uaf:
https://lore.kernel.org/netdev/20260909-vsock-connect-reset-closing-v1-1-50298b9ccfbf@rbox.co/
It's incomplete, but I'm not sure how to follow up. Please see the
below---comment.

thanks,
Michal