Re: [PATCH v6 1/3] vsock: don't check the listener's sk_err in vsock_accept()
Stefano Garzarella <[email protected]>
| Newsgroups | dev.linux.lists.virtualization,org.kernel.vger.linux-kernel,org.kernel.vger.netdev |
|---|---|
| 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]>