netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Boeuf, Sebastien" <sebastien.boeuf@intel.com>
To: "netdev@vger.kernel.org" <netdev@vger.kernel.org>
Cc: "stefanha@redhat.com" <stefanha@redhat.com>,
	"sgarzare@redhat.com" <sgarzare@redhat.com>,
	"davem@davemloft.net" <davem@davemloft.net>
Subject: [PATCH] net: virtio_vsock: Fix race condition between bind and listen
Date: Thu, 13 Feb 2020 09:16:11 +0000	[thread overview]
Message-ID: <668b0eda8823564cd604b1663dc53fbaece0cd4e.camel@intel.com> (raw)

From 2f1276d02f5a12d85aec5adc11dfe1eab7e160d6 Mon Sep 17 00:00:00 2001
From: Sebastien Boeuf <sebastien.boeuf@intel.com>
Date: Thu, 13 Feb 2020 08:50:38 +0100
Subject: [PATCH] net: virtio_vsock: Fix race condition between bind and listen

Whenever the vsock backend on the host sends a packet through the RX
queue, it expects an answer on the TX queue. Unfortunately, there is one
case where the host side will hang waiting for the answer and will
effectively never recover.

This issue happens when the guest side starts binding to the socket,
which insert a new bound socket into the list of already bound sockets.
At this time, we expect the guest to also start listening, which will
trigger the sk_state to move from TCP_CLOSE to TCP_LISTEN. The problem
occurs if the host side queued a RX packet and triggered an interrupt
right between the end of the binding process and the beginning of the
listening process. In this specific case, the function processing the
packet virtio_transport_recv_pkt() will find a bound socket, which means
it will hit the switch statement checking for the sk_state, but the
state won't be changed into TCP_LISTEN yet, which leads the code to pick
the default statement. This default statement will only free the buffer,
while it should also respond to the host side, by sending a packet on
its TX queue.

In order to simply fix this unfortunate chain of events, it is important
that in case the default statement is entered, and because at this stage
we know the host side is waiting for an answer, we must send back a
packet containing the operation VIRTIO_VSOCK_OP_RST.

Signed-off-by: Sebastien Boeuf <sebastien.boeuf@intel.com>
---
 net/vmw_vsock/virtio_transport_common.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
index e5ea29c6bca7..909334d58328 100644
--- a/net/vmw_vsock/virtio_transport_common.c
+++ b/net/vmw_vsock/virtio_transport_common.c
@@ -1143,6 +1143,7 @@ void virtio_transport_recv_pkt(struct virtio_transport *t,
 		virtio_transport_free_pkt(pkt);
 		break;
 	default:
+		(void)virtio_transport_reset_no_sock(t, pkt);
 		virtio_transport_free_pkt(pkt);
 		break;
 	}
-- 
2.20.1

---------------------------------------------------------------------
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: "Les Montalets"- 2, rue de Paris, 
92196 Meudon Cedex, France
Registration Number:  302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

             reply	other threads:[~2020-02-13  9:16 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-13  9:16 Boeuf, Sebastien [this message]
2020-02-13  9:41 ` [PATCH] net: virtio_vsock: Fix race condition between bind and listen Stefano Garzarella
2020-02-13  9:51   ` Boeuf, Sebastien
2020-02-13 10:22     ` Stefano Garzarella
2020-02-13 10:44       ` Boeuf, Sebastien
2020-02-13 11:02         ` Stefano Garzarella
2020-02-13 11:13           ` Boeuf, Sebastien
2020-02-13 11:22             ` Stefano Garzarella
2020-02-13 11:39         ` Stefan Hajnoczi
2020-02-13 13:04           ` Stefano Garzarella
2020-02-13 16:51             ` Boeuf, Sebastien
2020-02-13 17:14               ` Stefano Garzarella
2020-02-13 17:18                 ` Boeuf, Sebastien

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=668b0eda8823564cd604b1663dc53fbaece0cd4e.camel@intel.com \
    --to=sebastien.boeuf@intel.com \
    --cc=davem@davemloft.net \
    --cc=netdev@vger.kernel.org \
    --cc=sgarzare@redhat.com \
    --cc=stefanha@redhat.com \
    /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;
as well as URLs for NNTP newsgroup(s).