Netdev List
 help / color / mirror / Atom feed
* [PATCH] vsock: use sock_error() to consume sk_err after connect timeout
@ 2026-07-19 21:57 Nguyen Dinh Phi
  2026-07-20  8:17 ` Stefano Garzarella
  0 siblings, 1 reply; 3+ messages in thread
From: Nguyen Dinh Phi @ 2026-07-19 21:57 UTC (permalink / raw)
  To: Stefano Garzarella, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Simon Horman
  Cc: Nguyen Dinh Phi, syzbot+1b2c9c4a0f8708082678, virtualization,
	netdev, linux-kernel

After vsock_connect() exits the wait loop due to sk->sk_err being
set, the error was read but not cleared. This left sk->sk_err set
for subsequent operations.
Switch to sock_error() which atomically reads and clears sk->sk_err,
so the error is consumed when returned.

Signed-off-by: Nguyen Dinh Phi <phind.uet@gmail.com>
Reported-by: syzbot+1b2c9c4a0f8708082678@syzkaller.appspotmail.com
---
 net/vmw_vsock/af_vsock.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
index 622dbd046799..43eddc33ed12 100644
--- a/net/vmw_vsock/af_vsock.c
+++ b/net/vmw_vsock/af_vsock.c
@@ -1847,14 +1847,11 @@ static int vsock_connect(struct socket *sock, struct sockaddr_unsized *addr,
 		prepare_to_wait(sk_sleep(sk), &wait, TASK_INTERRUPTIBLE);
 	}
 
-	if (sk->sk_err) {
-		err = -sk->sk_err;
+	err = sock_error(sk);
+	if (err) {
 		sk->sk_state = TCP_CLOSE;
 		sock->state = SS_UNCONNECTED;
-	} else {
-		err = 0;
 	}
-
 out_wait:
 	finish_wait(sk_sleep(sk), &wait);
 out:
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] vsock: use sock_error() to consume sk_err after connect timeout
  2026-07-19 21:57 [PATCH] vsock: use sock_error() to consume sk_err after connect timeout Nguyen Dinh Phi
@ 2026-07-20  8:17 ` Stefano Garzarella
  2026-07-20 17:34   ` Phi Nguyen
  0 siblings, 1 reply; 3+ messages in thread
From: Stefano Garzarella @ 2026-07-20  8:17 UTC (permalink / raw)
  To: Nguyen Dinh Phi
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, syzbot+1b2c9c4a0f8708082678, virtualization, netdev,
	linux-kernel

On Mon, Jul 20, 2026 at 05:57:47AM +0800, Nguyen Dinh Phi wrote:
>After vsock_connect() exits the wait loop due to sk->sk_err being
>set, the error was read but not cleared. This left sk->sk_err set
>for subsequent operations.

So, is this a fix? If yes, we should put a Fixes tag.

Also, can you describe how to trigger the issue?

Because I see this in vsock_connect(), so I thought it was in some way 
already handled:

		/* sk_err might have been set as a result of an earlier
		 * (failed) connect attempt.
		 */
		sk->sk_err = 0;

>Switch to sock_error() which atomically reads and clears sk->sk_err,
>so the error is consumed when returned.
>
>Signed-off-by: Nguyen Dinh Phi <phind.uet@gmail.com>
>Reported-by: syzbot+1b2c9c4a0f8708082678@syzkaller.appspotmail.com

Can you explain how this patch fixes that issue?
(this should be the first information to be put in the commit message 
IMHO)

I'd like to understand better if this is a fix of real bug or just an 
improvement to the code (which is fine by me).

Thanks,
Stefano

>---
> net/vmw_vsock/af_vsock.c | 7 ++-----
> 1 file changed, 2 insertions(+), 5 deletions(-)
>
>diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
>index 622dbd046799..43eddc33ed12 100644
>--- a/net/vmw_vsock/af_vsock.c
>+++ b/net/vmw_vsock/af_vsock.c
>@@ -1847,14 +1847,11 @@ static int vsock_connect(struct socket *sock, struct sockaddr_unsized *addr,
> 		prepare_to_wait(sk_sleep(sk), &wait, TASK_INTERRUPTIBLE);
> 	}
>
>-	if (sk->sk_err) {
>-		err = -sk->sk_err;
>+	err = sock_error(sk);
>+	if (err) {
> 		sk->sk_state = TCP_CLOSE;
> 		sock->state = SS_UNCONNECTED;
>-	} else {
>-		err = 0;
> 	}
>-
> out_wait:
> 	finish_wait(sk_sleep(sk), &wait);
> out:
>-- 
>2.53.0
>


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] vsock: use sock_error() to consume sk_err after connect timeout
  2026-07-20  8:17 ` Stefano Garzarella
@ 2026-07-20 17:34   ` Phi Nguyen
  0 siblings, 0 replies; 3+ messages in thread
From: Phi Nguyen @ 2026-07-20 17:34 UTC (permalink / raw)
  To: Stefano Garzarella
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, syzbot+1b2c9c4a0f8708082678, virtualization, netdev,
	linux-kernel

On 7/20/2026 4:17 PM, Stefano Garzarella wrote:
> On Mon, Jul 20, 2026 at 05:57:47AM +0800, Nguyen Dinh Phi wrote:
>> After vsock_connect() exits the wait loop due to sk->sk_err being
>> set, the error was read but not cleared. This left sk->sk_err set
>> for subsequent operations.
> 
> So, is this a fix? If yes, we should put a Fixes tag.
> 
> Also, can you describe how to trigger the issue?
> 
> Because I see this in vsock_connect(), so I thought it was in some way 
> already handled:
> 
>          /* sk_err might have been set as a result of an earlier
>           * (failed) connect attempt.
>           */
>          sk->sk_err = 0;
> 
This only handles the case where the function following the failed 
connect is another connect() call.

>> Switch to sock_error() which atomically reads and clears sk->sk_err,
>> so the error is consumed when returned.
>>
>> Signed-off-by: Nguyen Dinh Phi <phind.uet@gmail.com>
>> Reported-by: syzbot+1b2c9c4a0f8708082678@syzkaller.appspotmail.com
> 
> Can you explain how this patch fixes that issue?
> (this should be the first information to be put in the commit message IMHO)
> 
> I'd like to understand better if this is a fix of real bug or just an 
> improvement to the code (which is fine by me).
> 
> Thanks,
> Stefano
> 
Here are the steps of the syzkaller reproducer:

   r0 = socket(AF_VSOCK, SOCK_STREAM, 0) 

   bind(r0, {VMADDR_CID_ANY, PORT}) 

   connect(r0, {VMADDR_CID_LOCAL, PORT}) 

   listen(r0, backlog)
  

   r1 = socket(AF_VSOCK, SOCK_STREAM, 0) 

   connect(r1, {VMADDR_CID_LOCAL, PORT})

   connect(r0 -> self) -> -1, EPROTO 

   listen(r0)          -> 0 

   connect(r1 -> r0)   -> 0 

   accept(r0)          -> -1, EPROTO

Basically, it creates a socket (r0) and triggers a self-connect after 
binding it. This self-connect fails with EPROTO because it loops back to 
r0 while the socket is still in the TCP_SYN_SENT state, causing it to be 
incorrectly dispatched to the connecting-client path. The unexpected 
packet type encountered there sets sk_err to EPROTO.

After that, it invokes a listen() call on the same socket. This listen() 
call succeeds because the kernel's listening path never inspects or 
clears sk_err. Then, a new socket (r1) is created as a normal client and 
connects to r0. However, vsock_accept() rejects this incoming connection 
because the listener's sk_err still holds the EPROTO error from the 
earlier failed self-connect.

This rejection causes the child socket created for r1's connection to 
never be freed on virtio or hyperv transports; only the VMCI transport 
implements pending_work to revisit and clean up a rejected socket
This patch will prevent the rejection branch to occur in this scenario.

I think we might schedule the cleanup worker to run in the rejection 
path for these transports as well.

Thanks,
Phi

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-07-20 17:34 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-19 21:57 [PATCH] vsock: use sock_error() to consume sk_err after connect timeout Nguyen Dinh Phi
2026-07-20  8:17 ` Stefano Garzarella
2026-07-20 17:34   ` Phi Nguyen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox