Netdev List
 help / color / mirror / Atom feed
From: Stefano Garzarella <sgarzare@redhat.com>
To: "Bartłomiej Dmitruk" <bartlomiej.dmitruk@isec.pl>
Cc: "K . Y . Srinivasan" <kys@microsoft.com>,
	 Haiyang Zhang <haiyangz@microsoft.com>,
	Wei Liu <wei.liu@kernel.org>, Dexuan Cui <decui@microsoft.com>,
	 Long Li <longli@microsoft.com>,
	"David S . Miller" <davem@davemloft.net>,
	 Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>,
	 Paolo Abeni <pabeni@redhat.com>, Simon Horman <horms@kernel.org>,
	 "Michael S . Tsirkin" <mst@redhat.com>,
	linux-hyperv@vger.kernel.org, virtualization@lists.linux.dev,
	 netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] vsock/hv_sock: don't finish a connection on a socket being released
Date: Tue, 22 Sep 2026 14:33:04 +0200	[thread overview]
Message-ID: <arJ0NhXGdCOqRjlj@sgarzare-redhat> (raw)
In-Reply-To: <20260919123128.28868-1-bartlomiej.dmitruk@isec.pl>

On Sat, Sep 19, 2026 at 02:31:27PM +0200, Bartłomiej Dmitruk wrote:
>hvs_open_connection() only checks sk->sk_state (TCP_LISTEN for a
>host-initiated connection, TCP_SYN_SENT for a guest-initiated one) before
>proceeding. __vsock_release() sets sk->sk_shutdown = SHUTDOWN_MASK and, for
>a listener, flushes the accept queue, while leaving sk_state unchanged. A
>VMBUS channel offer that races close() therefore still passes the state
>check and either enqueues a child onto the already-flushed accept queue
>(conn_from_host) or completes a connection on a socket being torn down
>(!conn_from_host); the socket and its VMBUS channel then leak.
>
>The virtio transport guards the listener case in
>virtio_transport_recv_listen(); hv_sock has no equivalent. Add a
>sk_shutdown == SHUTDOWN_MASK check to the common path right after
>lock_sock(), so both the host- and guest-initiated paths bail out.
>hv_sock holds lock_sock(sk) across hvs_open_connection(), so the check is
>race-free.
>
>This was found by code inspection; I do not have Hyper-V hardware to test on.
>
>Fixes: ae0078fcf0a5 ("hv_sock: implements Hyper-V transport for Virtual Sockets (AF_VSOCK)")
>Signed-off-by: Bartłomiej Dmitruk <bartlomiej.dmitruk@isec.pl>
>---
>v2: move the SHUTDOWN_MASK check to the common checks after lock_sock() so it
>    also covers the guest-initiated (!conn_from_host) path; add Fixes tag
>    (Stefano Garzarella).
>v1: https://lore.kernel.org/netdev/20260917220156.56015-1-bartlomiej.dmitruk@isec.pl/
>

About v1, I see sashiko reporting some pre-existing issues
https://lore.kernel.org/netdev/179003264046.2160803.3176959842906352107@kernel.org/

The critical one seems real, can you check?
Maybe it should be fixed before this change.

Stefano

>diff --git a/net/vmw_vsock/hyperv_transport.c b/net/vmw_vsock/hyperv_transport.c
>--- a/net/vmw_vsock/hyperv_transport.c
>+++ b/net/vmw_vsock/hyperv_transport.c
>@@ -322,6 +322,15 @@
> 	    (!conn_from_host && sk->sk_state != TCP_SYN_SENT))
> 		goto out;
>
>+	/* __vsock_release() may have already set sk_shutdown = SHUTDOWN_MASK
>+	 * (and, for a listener, flushed the accept queue) while leaving the
>+	 * socket in TCP_LISTEN/TCP_SYN_SENT.  Proceeding would enqueue a child
>+	 * onto a released listener, or complete a connection on a released
>+	 * socket, leaking it and its VMBUS channel.
>+	 */
>+	if (sk->sk_shutdown == SHUTDOWN_MASK)
>+		goto out;
>+
> 	if (conn_from_host) {
> 		if (sk_acceptq_is_full(sk))
> 			goto out;
>


  reply	other threads:[~2026-09-22 12:33 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-19 12:31 [PATCH v2] vsock/hv_sock: don't finish a connection on a socket being released Bartłomiej Dmitruk
2026-09-22 12:33 ` Stefano Garzarella [this message]
2026-09-23 12:46 ` netdev-bot+sashiko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=arJ0NhXGdCOqRjlj@sgarzare-redhat \
    --to=sgarzare@redhat.com \
    --cc=bartlomiej.dmitruk@isec.pl \
    --cc=davem@davemloft.net \
    --cc=decui@microsoft.com \
    --cc=edumazet@google.com \
    --cc=haiyangz@microsoft.com \
    --cc=horms@kernel.org \
    --cc=kuba@kernel.org \
    --cc=kys@microsoft.com \
    --cc=linux-hyperv@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=longli@microsoft.com \
    --cc=mst@redhat.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=virtualization@lists.linux.dev \
    --cc=wei.liu@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox