Re: [PATCH] vsock: fix memory leak of rejected child sockets in vsock_accept()

[Stefano Garzarella <sgarzare@redhat.com>]

On Fri, Aug 07, 2026 at 11:48:04AM +0000, Chaithanya Lagisetty wrote:
>When a listener socket carries an error (e.g. sk_err set by a connect()
>issued on the socket before listen()), vsock_accept() dequeues the child
>from the accept queue but rejects it. Previously it only marked the child
>as rejected and relied on vsock_pending_work() to clean it up. However,
>rejected child sockets created through virtio_transport and vsock_loopback
>never reach that cleanup path, causing the child socket, along with its
>LSM blob and transport-specific state, to leak permanently.
>
>Fix this by releasing the child's references directly in vsock_accept()
>on the reject path: remove it from the connected table and drop the
>references taken by sk_alloc(), __vsock_insert_connected() and
>vsock_enqueue_accept(), so the socket reaches vsock_sk_destruct() and is
>freed. The now-unused 'rejected' flag and its handling in
>vsock_pending_work() are removed.
>
>Fixes: 06a8fc78367d ("VSOCK: Introduce virtio_vsock_common.ko")
>Reported-by: syzbot+53515d23498d641e21ea@syzkaller.appspotmail.com
>Closes: https://syzkaller.appspot.com/bug?extid=53515d23498d641e21ea
>Signed-off-by: Chaithanya Lagisetty <nagachaithanya9911@gmail.com>
>---
> include/net/af_vsock.h   |  4 +---
> net/vmw_vsock/af_vsock.c | 38 ++++++++++++++++++++------------------
> 2 files changed, 21 insertions(+), 21 deletions(-)

Thanks for this, this is something similar of what I suggested some days 
ago to Phi in 
https://lore.kernel.org/netdev/anLuRE4ix5-BZ7-t@sgarzare-redhat/

Phi's work also includes more cleanups, so let's avoid duplicated work.

Please test Phi's v5 series that should include something similar, plus 
other stuff that should fix exaclty the same report.

Stefano

>
>diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h
>index 30046a3c20f7..b70cea7f754f 100644
>--- a/include/net/af_vsock.h
>+++ b/include/net/af_vsock.h
>@@ -53,12 +53,10 @@ struct vsock_sock {
> 	 * for connection requests are placed in the pending list until they
> 	 * are connected, at which point they are put in the accept queue list
> 	 * so they can be accepted in accept().  If accept() cannot accept the
>-	 * connection, it is marked as rejected so the cleanup function knows
>-	 * to clean up the socket.
>+	 * connection, the child is cleaned up directly in vsock_accept().
> 	 */
> 	struct list_head pending_links;
> 	struct list_head accept_queue;
>-	bool rejected;
> 	struct delayed_work connect_work;
> 	struct delayed_work pending_work;
> 	struct delayed_work close_work;
>diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
>index 622dbd046799..2084c88ac836 100644
>--- a/net/vmw_vsock/af_vsock.c
>+++ b/net/vmw_vsock/af_vsock.c
>@@ -39,9 +39,9 @@
>  * from the listener socket's pending list and enqueued in the listener
>  * socket's accept queue.  Callers of accept(2) will accept connected sockets
>  * from the listener socket's accept queue.  If the socket cannot be accepted
>- * for some reason then it is marked rejected.  Once the connection is
>- * accepted, it is owned by the user process and the responsibility for cleanup
>- * falls with that user process.
>+ * for some reason then it is cleaned up directly in vsock_accept().  Once the
>+ * connection is accepted, it is owned by the user process and the
>+ * responsibility for cleanup falls with that user process.
>  *
>  * - It is possible that these pending sockets will never reach the connected
>  * state; in fact, we may never receive another packet after the connection
>@@ -49,9 +49,7 @@
>  * future, after some amount of time passes where a connection should have been
>  * established.  This function ensures that the socket is off all lists so it
>  * cannot be retrieved, then drops all references to the socket so it is cleaned
>- * up (sock_put() -> sk_free() -> our sk_destruct implementation).  Note this
>- * function will also cleanup rejected sockets, those that reach the connected
>- * state but leave it before they have been accepted.
>+ * up (sock_put() -> sk_free() -> our sk_destruct implementation).
>  *
>  * - Lock ordering for pending or accept queue sockets is:
>  *
>@@ -774,11 +772,11 @@ static void vsock_pending_work(struct work_struct *work)
>
> 	if (vsock_is_pending(sk)) {
> 		vsock_remove_pending(listener, sk);
>-	} else if (!vsk->rejected) {
>-		/* We are not on the pending list and accept() did not reject
>-		 * us, so we must have been accepted by our user process.  We
>-		 * just need to drop our references to the sockets and be on
>-		 * our way.
>+	} else {
>+		/* We are not on the pending list, so we must have been accepted
>+		 * by our user process (rejected sockets are cleaned up directly
>+		 * in vsock_accept()).  We just need to drop our references to
>+		 * the sockets and be on our way.
> 		 */
> 		cleanup = false;
> 		goto out;
>@@ -942,7 +940,6 @@ static struct sock *__vsock_create(struct net *net,
> 	vsk->listener = NULL;
> 	INIT_LIST_HEAD(&vsk->pending_links);
> 	INIT_LIST_HEAD(&vsk->accept_queue);
>-	vsk->rejected = false;
> 	vsk->sent_request = false;
> 	vsk->ignore_connecting_rst = false;
> 	WRITE_ONCE(vsk->peer_shutdown, 0);
>@@ -1919,14 +1916,19 @@ static int vsock_accept(struct socket *sock, struct socket *newsock,
> 		vconnected = vsock_sk(connected);
>
> 		/* If the listener socket has received an error, then we should
>-		 * reject this socket and return.  Note that we simply mark the
>-		 * socket rejected, drop our reference, and let the cleanup
>-		 * function handle the cleanup; the fact that we found it in
>-		 * the listener's accept queue guarantees that the cleanup
>-		 * function hasn't run yet.
>+		 * reject this socket and return.  The child was found on the
>+		 * listener's accept queue, so it still holds the references
>+		 * taken by sk_alloc(), __vsock_insert_connected() and
>+		 * vsock_enqueue_accept().  Drop them here so the socket is
>+		 * destroyed.  We cannot defer this to vsock_pending_work():
>+		 * rejected child sockets created through virtio_transport and
>+		 * vsock_loopback are not cleaned up by that worker, so the
>+		 * child would otherwise leak permanently.
> 		 */
> 		if (err) {
>-			vconnected->rejected = true;
>+			vsock_remove_connected(vconnected);
>+			connected->sk_state = TCP_CLOSE;
>+			sock_put(connected);
> 		} else {
> 			newsock->state = SS_CONNECTED;
> 			sock_graft(connected, newsock);
>-- 
>2.43.0
>