[PATCH v2] vsock: use sock_error() to consume sk_err after a

[PATCH v2] vsock: use sock_error() to consume sk_err after a

[phind.uet]

From: Nguyen Dinh Phi <phind.uet@gmail.com>

Syzbot report an issue which can be reproduced with these steps:
   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

Fix the issue by using sock_error() to read the sk_err to prevent the
rejection branch from occurring  in this scenario.

sock_error() atomically reads and clears sk_err, ensuring the error is
consumed when vsock_connect() returns and cannot affect subsequent
operations on the same socket. This matches the established pattern
used by other protocol connect() implementations in the network
stack like __inet_stream_connect(), tipc_wait_for_connect()...

Reported-by: syzbot+1b2c9c4a0f8708082678@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=1b2c9c4a0f8708082678
Fixes: d021c344051af ("VSOCK: Introduce VM Sockets")
Signed-off-by: Nguyen Dinh Phi <phind.uet@gmail.com>
---
V2: Add reproducer steps to commit message.

 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

Re: [PATCH v2] vsock: use sock_error() to consume sk_err after a

[mawupeng]

On 周一 2026-7-27 15:13, phind.uet@gmail.com wrote:
> From: Nguyen Dinh Phi <phind.uet@gmail.com>
> 
> Syzbot report an issue which can be reproduced with these steps:
>    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
> 
> Fix the issue by using sock_error() to read the sk_err to prevent the
> rejection branch from occurring  in this scenario.
> 
> sock_error() atomically reads and clears sk_err, ensuring the error is
> consumed when vsock_connect() returns and cannot affect subsequent
> operations on the same socket. This matches the established pattern
> used by other protocol connect() implementations in the network
> stack like __inet_stream_connect(), tipc_wait_for_connect()...
> 
> Reported-by: syzbot+1b2c9c4a0f8708082678@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=1b2c9c4a0f8708082678
> Fixes: d021c344051af ("VSOCK: Introduce VM Sockets")
> Signed-off-by: Nguyen Dinh Phi <phind.uet@gmail.com>

Thanks for your patch.

Test-by: Wupeng Ma <mawupeng1@huawei.com>

> ---
> V2: Add reproducer steps to commit message.
> 
>  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:

Re: [PATCH v2] vsock: use sock_error() to consume sk_err after a

[Stefano Garzarella]

commit title seems truncated, can you check?

On Mon, Jul 27, 2026 at 03:13:02PM +0800, phind.uet@gmail.com wrote:
>From: Nguyen Dinh Phi <phind.uet@gmail.com>
>
>Syzbot report an issue which can be reproduced with these steps:
>   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

I spent some time to understand this, what about changing in this way
(or something similar):

     r0 = socket(AF_VSOCK, SOCK_STREAM, 0)
     bind(r0, {VMADDR_CID_ANY, PORT})
     connect(r0, {VMADDR_CID_LOCAL, PORT})   -> -1, EPROTO  (self-connect)
     listen(r0, backlog)                     -> 0
     r1 = socket(AF_VSOCK, SOCK_STREAM, 0)
     connect(r1, {VMADDR_CID_LOCAL, PORT})   -> 0
     accept(r0)                              -> -1, EPROTO  (stale sk_err)

>
>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
>
>Fix the issue by using sock_error() to read the sk_err to prevent the
>rejection branch from occurring  in this scenario.
>
>sock_error() atomically reads and clears sk_err, ensuring the error is
>consumed when vsock_connect() returns and cannot affect subsequent
>operations on the same socket. This matches the established pattern
>used by other protocol connect() implementations in the network
>stack like __inet_stream_connect(), tipc_wait_for_connect()...
>
>Reported-by: syzbot+1b2c9c4a0f8708082678@syzkaller.appspotmail.com
>Closes: https://syzkaller.appspot.com/bug?extid=1b2c9c4a0f8708082678
>Fixes: d021c344051af ("VSOCK: Introduce VM Sockets")
>Signed-off-by: Nguyen Dinh Phi <phind.uet@gmail.com>
>---
>V2: Add reproducer steps to commit message.
>
> 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);

Should we do the same in other paths (e.g. send/recv) as well in a 
follwup patch or in a series?

The patch itself LGTM.

Thanks,
Stefano

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

Re: [PATCH v2] vsock: use sock_error() to consume sk_err after a

[Stefano Garzarella]

On Tue, Jul 28, 2026 at 11:14:31AM +0800, mawupeng wrote:
>
>
>On 周一 2026-7-27 15:13, phind.uet@gmail.com wrote:
>> From: Nguyen Dinh Phi <phind.uet@gmail.com>
>>
>> Syzbot report an issue which can be reproduced with these steps:
>>    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
>>
>> Fix the issue by using sock_error() to read the sk_err to prevent the
>> rejection branch from occurring  in this scenario.
>>
>> sock_error() atomically reads and clears sk_err, ensuring the error is
>> consumed when vsock_connect() returns and cannot affect subsequent
>> operations on the same socket. This matches the established pattern
>> used by other protocol connect() implementations in the network
>> stack like __inet_stream_connect(), tipc_wait_for_connect()...
>>
>> Reported-by: syzbot+1b2c9c4a0f8708082678@syzkaller.appspotmail.com
>> Closes: https://syzkaller.appspot.com/bug?extid=1b2c9c4a0f8708082678
>> Fixes: d021c344051af ("VSOCK: Introduce VM Sockets")
>> Signed-off-by: Nguyen Dinh Phi <phind.uet@gmail.com>
>
>Thanks for your patch.
>
>Test-by: Wupeng Ma <mawupeng1@huawei.com>

Thanks for testing, just a note, the right tag should be Tested-by:

Anyway, @Phi can you bring this with the right tag to the v3 if the code 
will not change?

Thanks,
Stefano