From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-oa1-f49.google.com (mail-oa1-f49.google.com [209.85.160.49]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 624A24A68B0 for ; Wed, 9 Sep 2026 22:44:45 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.160.49 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1788993893; cv=none; b=rWUjDgNZ8HLKVncdaZlVE+AVvcM/Yv5gZOqgLICOHqcd7pdXWi7GPYMzmCat+N7gYVaaMd3j8mx7B7IOBkWax3wG1sTnsIFgAYFyG1gUZyfWUJjC930wEkILfhAdWuV4x1fe4PGDWHK3SKuqcg2H2o9pzJrE8al//J74sFQrvpc= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1788993893; c=relaxed/simple; bh=T05NvHvBe6jr/5zMRdBfL+ppGgLwUsijA+cXkh/ecoU=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=YIJmDCKWPD8rS0kYNzJvnD91eC0k2/k7ZfGJ6aFAh9PbAWkYXAjtSinuD8Reh0BSDFVoYUzcgBAe2SgQsvZ+y5mt2DgnTq7We4M/jBrZT46PcpFoBDuwZnobQMRyZZSOYfHtXR2u8AMa3EdZfuu10ZfhxX+aCq/u8rdr7NZd24I= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=gKNiUhgp; arc=none smtp.client-ip=209.85.160.49 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="gKNiUhgp" Received: by mail-oa1-f49.google.com with SMTP id 586e51a60fabf-46ae77dcd0cso2218029fac.1 for ; Wed, 09 Sep 2026 15:44:42 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1788993878; x=1789598678; darn=vger.kernel.org; h=in-reply-to:content-disposition:content-type:mime-version :references:message-id:subject:cc:to:from:date:from:to:cc:subject :date:message-id:reply-to:content-type; bh=C9BgiwK82Q/ir0W+AfKhqhG0iwn5MN55494Vi6I3LH0=; b=gKNiUhgpG1qL5ZzBRD8krH6Qqp+ypEh8XaB2rnRO5ve1GzJLGgkdeU5ZsD/S3pOASe +9XBsy5sxi0eqbU1T2v1WrhZvbCeU6vRqqTeRkTMBgsyWLnf8uDCovf18Sp2eiNBes3b rdAIaWVvDa1aNujhmpGxoSGL0ICxE5MAmxUiWP3QKKVBdrGIkxk7MeXOnh+PiZIUkMvm wEUp3dEg/34dkF0JoomvZ8fzVnnl54joH9orNIsoHIUmFxytJhJzbayVbKIvKKBQIBkR BwOu6a6rSqhzaV3+22hR9o/zeOnLgEtYE5m8a+G6DDoj8gU5wLnp+lASsZJjUMjtlCu1 8zQQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1788993878; x=1789598678; h=in-reply-to:content-disposition:content-type:mime-version :references:message-id:subject:cc:to:from:date:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to :content-type; bh=C9BgiwK82Q/ir0W+AfKhqhG0iwn5MN55494Vi6I3LH0=; b=TYo7VUy/AXbxnor28/nxHwyelyM4MuQiytzEYuvJLxiwr1BsXBcNMnW2Sn84cc5I6f nJljZWJZVYWKHtqZ1NmhWFy6zMmWTwOlEENm3rtii9dNna3xqKzwOAYeelCgeWUTH0tv ln5JTGRDv3Q/6j/2ijh3qKvz17muH4vjZc8QfFhmjL0HdBoeQdVpFnTSCyG5VpSj0vIp Jf+A2Q4NhRKka0Uhdyzb6RFf6jImYmb2vWLSAywfYcwCIeWbewkz8oWLZ5lG+/xa8ddY frkSRwh+w/F1JKJICk39dRmJE03YQLNOTtX4Ofe8PovkN7qX0odZTub1QrPNYHce63yL pFFg== X-Forwarded-Encrypted: i=1; AKwUvBzUyGfHdOg2gcNATOKua+nUueC5EGsSs5GJslllUQ7xlFyQESwJx+rsn/rE39U5oAXXECVHMzehOVcUr3E=@vger.kernel.org X-Gm-Message-State: AFuF++kHTcMlJ1tqR7v8UyHwyLHzaWOPhK66nqapIqwF2dB9KFR+qh0J xVhT4JUg6xf96ZGhE4Anh31LfrRbFtZpyYD4NY1M0clpKxtv+IZj7e6D X-Gm-Gg: AYBFou3ZS/jK4t4nDSOMGQjLQpT6kUzDBsB+JNGGbA3O8UaLN3LbYztiHO636ULu2lO jcTiHrmrANLOyF6uoyuybei7R8IVHRegNaQiFRs2WPFq71InzbR1RsKvad7waeUH3SPp2IpqD3H O5VT4E59qZyD1plMaz8aYYSEpSCLieapq9TkO38o3xA6mb70S8f9sPi09+1BOnKS6iXYs27hgqf tUu+rcdLQH+kAIyA60FSH/j4Nr2w7MOv2Kq4RrtjF00G0rFUGD2hbtKeQsdIWELRJPAPLtvDBCn QmYbaszxNfTOwFiw9QRrNiB+7XnWqNS4EzLajHlgA9HUNgGqizHtpGN8F8MLoGCq5/+Og2FoBaq OmvRy8YZKf8cSbBQKuOmlL3y/QI3JRh4HEMR08UujGodoCFr4ILQZkHGGCAhq5nDu2Ih9Ge8UmL phKdQspWb6wiXN/ESh7mk+w7xQLqLWOy6kY3Q2ng0Wysmb4PRgtKDjneCH9N6IRiJOY7Bft50Zt r4PE4cjeJOZVmBdpopW X-Received: by 2002:a05:6820:80f:b0:6b1:4250:6a43 with SMTP id 006d021491bc7-6b6fd2ce67fmr21915798eaf.17.1788993877718; Wed, 09 Sep 2026 15:44:37 -0700 (PDT) Received: from devvm29614.prn0.facebook.com ([2a03:2880:ff:57::]) by smtp.gmail.com with ESMTPSA id 006d021491bc7-6bebe8d3f3fsm1833474eaf.10.2026.09.09.15.44.35 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 09 Sep 2026 15:44:37 -0700 (PDT) Date: Wed, 9 Sep 2026 15:44:33 -0700 From: Bobby Eshleman To: Michal Luczaj Cc: Stefano Garzarella , "David S. Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , Simon Horman , Dmitry Torokhov , Andy King , George Zhang , virtualization@lists.linux.dev, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, Hyunwoo Kim Subject: Re: [PATCH net] vsock: Do not reset a TCP_CLOSING socket Message-ID: References: <20260909-vsock-connect-reset-closing-v1-1-50298b9ccfbf@rbox.co> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20260909-vsock-connect-reset-closing-v1-1-50298b9ccfbf@rbox.co> On Wed, Sep 09, 2026 at 11:58:26PM +0200, Michal Luczaj wrote: > Ensure connect() resets the socket only if it has never been established. > Handle the previously overlooked TCP_ESTABLISHED -> TCP_CLOSING > transition (on VIRTIO_VSOCK_OP_RST), which could race with the connect > loop. > > Resetting a socket that is still present in connected_table can lead to > memory corruption. The reporter noted lost transports for in-flight skbs, > and I have reproduced crashes caused by re-insertion into connected_table. > > list_add double add: new=, prev=, next=. > kernel BUG at lib/list_debug.c:35! > Oops: invalid opcode: 0000 [#1] SMP KASAN NOPTI > Workqueue: vsock-loopback vsock_loopback_work > RIP: 0010:__list_add_valid_or_report+0x11f/0x130 > Call Trace: > vsock_insert_connected.cold+0xe/0x13 > virtio_transport_recv_pkt+0x10e9/0x1460 > vsock_loopback_work+0x305/0x480 > process_one_work+0xe4c/0x1560 > worker_thread+0x4f1/0xd60 > kthread+0x36e/0x470 > ret_from_fork+0x47b/0x6b0 > ret_from_fork_asm+0x1a/0x30 > > Drop the inaccurate comment above signal_pending(). This fix is > supplementary to commit 002541ef650b ("vsock: Ignore signal/timeout on > connect() if already established"). Details at Link. > > Fixes: d021c344051a ("VSOCK: Introduce VM Sockets") > Reported-by: Hyunwoo Kim > Link: https://lore.kernel.org/netdev/anzT1fREOSyHT99k@v4bel/ > Signed-off-by: Michal Luczaj > --- > Note that this is not a complete fix. connect()'s schedule_timeout() can > still race with two other functions that set sk_state = TCP_CLOSE while > keeping the socket in connected_table: > 1. vmci_transport_handle_detach(): no way for me to test, > 2. virtio_vsock_reset_sock(): tested by unbinding the driver > (/sys/bus/virtio/drivers/virtio_transport/unbind). > The latter appears easy to fix by adding __vsock_remove_connected() and > switching to a _safe iterator in vsock_for_each_connected_socket(). > --- > net/vmw_vsock/af_vsock.c | 14 ++++++-------- > 1 file changed, 6 insertions(+), 8 deletions(-) > > diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c > index f840498b58af..eec5dd6daebb 100644 > --- a/net/vmw_vsock/af_vsock.c > +++ b/net/vmw_vsock/af_vsock.c > @@ -1834,23 +1834,20 @@ static int vsock_connect(struct socket *sock, struct sockaddr_unsized *addr, > timeout = schedule_timeout(timeout); > lock_sock(sk); > > - /* Connection established. Whatever happens to socket once we > - * release it, that's not connect()'s concern. No need to go > + /* Connection was established. Whatever happens to socket once > + * we release it, that's not connect()'s concern. No need to go > * into signal and timeout handling. Call it a day. > * > * Note that allowing to "reset" an already established socket > * here is racy and insecure. > */ > - if (sk->sk_state == TCP_ESTABLISHED) > + if (sk->sk_state == TCP_ESTABLISHED || > + sk->sk_state == TCP_CLOSING) > break; > > /* If connection was _not_ established and a signal/timeout came > * to be, we want the socket's state reset. User space may want > * to retry. > - * > - * sk_state != TCP_ESTABLISHED implies that socket is not on > - * vsock_connected_table. We keep the binding and the transport > - * assigned. > */ > if (signal_pending(current) || timeout == 0) { > err = timeout == 0 ? -ETIMEDOUT : sock_intr_errno(timeout); > @@ -1875,7 +1872,8 @@ static int vsock_connect(struct socket *sock, struct sockaddr_unsized *addr, > } > > err = sock_error(sk); > - if (err) { > + if (err && > + sk->sk_state != TCP_ESTABLISHED && sk->sk_state != TCP_CLOSING) { If the OP_RESPONSE + a blast of OP_RW that pushes past the buffer limit arrives while we were scheduled out, we end up with sk_err = ENOBUFS here. Then I guess connect() returns an error, but sk_state/sock->state is still TCP_ESTABLISHED and SS_CONNECTED. If the user sees the error and tries connect() again, they just get -EISCONN back. Maybe the sock_error() needs to be moved within the conditional here, and then let subsequent calls return the error to the user (it looks sendmsg() at least will report it faithfully, but not sure about recvmsg() or the others). Best, Bobby