Re: [PATCH v6 1/3] vsock: don't check the listener's sk_err in vsock_accept()

Stefano Garzarella <[email protected]>
Newsgroups org.kernel.vger.netdev,dev.linux.lists.virtualization,org.kernel.vger.linux-kernel
Message-ID <an7ZiFM2nmb6kn2_@sgarzare-redhat>
On Fri, Aug 14, 2026 at 01:30:18AM +0800, Nguyen Dinh Phi wrote:
>Syzbot reported 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}) -> -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.
>
>For a non-blocking connect(), vsock_connect() may return -EINPROGRESS
>immediately, and vsock_connect_timeout() can later set sk->sk_err
>asynchronously.
>
>Since no vsock transport ever sets sk_err on a socket while it is in
>TCP_LISTEN state, checking it in vsock_accept() serves no purpose and
>only carries forward errors left behind by earlier, unrelated
>connection attempts on the same socket. Remove the checks so accept()
>no longer rejects valid incoming connections because of a stale
>error, which also avoids the resource leak described above.
>
>Fixes: d021c344051a ("VSOCK: Introduce VM Sockets")
>Reported-by: [email protected]
>Closes: https://syzkaller.appspot.com/bug?extid=1b2c9c4a0f8708082678
>Suggested-by: Michal Luczaj <[email protected]>
>Signed-off-by: Nguyen Dinh Phi <[email protected]>
>---
> net/vmw_vsock/af_vsock.c | 10 +++-------
> 1 file changed, 3 insertions(+), 7 deletions(-)
>
>diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
>index 622dbd046799..3cd5c3561be3 100644
>--- a/net/vmw_vsock/af_vsock.c
>+++ b/net/vmw_vsock/af_vsock.c
>@@ -1893,7 +1893,7 @@ static int vsock_accept(struct socket *sock, struct socket *newsock,
> 	timeout = sock_rcvtimeo(listener, arg->flags & O_NONBLOCK);
>
> 	while ((connected = vsock_dequeue_accept(listener)) == NULL &&
>-	       listener->sk_err == 0 && timeout != 0) {
>+		timeout != 0) {
                 ^
nit: I think the alignment is wrong, maybe if you need to respin you can 
fix it

Anyway, this LGTM

Reviewed-by: Stefano Garzarella <[email protected]>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.