xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: Andrew Cooper <andrew.cooper3@citrix.com>
To: Xen-devel <xen-devel@lists.xen.org>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>,
	Ian Jackson <Ian.Jackson@eu.citrix.com>,
	Ian Campbell <Ian.Campbell@citrix.com>,
	Stefano Stabellini <stefano.stabellini@citrix.com>
Subject: [PATCH v2 qemu-traditional] qemu: Fix race condition when opening ports
Date: Mon, 9 Dec 2013 13:27:23 +0000	[thread overview]
Message-ID: <1386595643-8639-1-git-send-email-andrew.cooper3@citrix.com> (raw)
In-Reply-To: <52A5C2E6.3080005@eu.citrix.com>

Two Qemus can race to bind the same VNC port.  It is valid for multiple
bind()s on the same socket to succeed, but only the first listen() will
succeed.

In the case that two Qemus are starting at the same time, and both trying to
grab the next free VNC port, the second one will fail with an EADDRINUSE, and
bail with a fatal error which renders the domain functionally useless.

In the case that listen() fails with EADDRINUSE, rebind the socket again and
try for the next port.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Ian Campbell <Ian.Campbell@citrix.com>
CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Stefano Stabellini <stefano.stabellini@citrix.com>
Release-acked-by: George Dunlap <george.dunlap@eu.citrix.com>

---

Changes in v2:
 * Fix commit message.

This fix has been in XenServer for two and a half years
---
 qemu-sockets.c |   23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/qemu-sockets.c b/qemu-sockets.c
index 9b9fa77..e54f6ad 100644
--- a/qemu-sockets.c
+++ b/qemu-sockets.c
@@ -163,6 +163,7 @@ int inet_listen(const char *str, char *ostr, int olen,
 
     /* create socket + bind */
     for (e = res; e != NULL; e = e->ai_next) {
+    rebind:
 	getnameinfo((struct sockaddr*)e->ai_addr,e->ai_addrlen,
 		    uaddr,INET6_ADDRSTRLEN,uport,32,
 		    NI_NUMERICHOST | NI_NUMERICSERV);
@@ -186,7 +187,20 @@ int inet_listen(const char *str, char *ostr, int olen,
                 if (sockets_debug)
                     fprintf(stderr,"%s: bind(%s,%s,%d): OK\n", __FUNCTION__,
                             inet_strfamily(e->ai_family), uaddr, inet_getport(e));
-                goto listen;
+                if (listen(slisten,1) == 0) {
+                    goto listened;
+                } else {
+                    int err = errno;
+
+                    perror("listen");
+                    closesocket(slisten);
+
+                    if (err == EADDRINUSE)
+                        goto rebind;
+                    freeaddrinfo(res);
+                    return -1;
+                }
+
             }
             try_next = to && (inet_getport(e) <= to + port_offset);
             if (!try_next || sockets_debug)
@@ -205,12 +219,7 @@ int inet_listen(const char *str, char *ostr, int olen,
     freeaddrinfo(res);
     return -1;
 
-listen:
-    if (listen(slisten,1) != 0) {
-        perror("listen");
-        closesocket(slisten);
-        return -1;
-    }
+listened:
     if (ostr) {
         if (e->ai_family == PF_INET6) {
             snprintf(ostr, olen, "[%s]:%d%s", uaddr,
-- 
1.7.10.4

  parent reply	other threads:[~2013-12-09 13:27 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-12-09 13:10 [PATCH qemu-traditional] qemu: Fix race condition when binding ports Andrew Cooper
2013-12-09 13:17 ` George Dunlap
2013-12-09 13:25   ` Andrew Cooper
2013-12-09 13:27   ` Andrew Cooper [this message]
2013-12-09 13:36     ` [PATCH v2 qemu-traditional] qemu: Fix race condition when opening ports Ian Campbell
2013-12-09 15:23     ` Frediano Ziglio
2013-12-09 15:50       ` Andrew Cooper
2013-12-10 15:28     ` Ian Jackson
2013-12-10 15:38       ` Frediano Ziglio

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=1386595643-8639-1-git-send-email-andrew.cooper3@citrix.com \
    --to=andrew.cooper3@citrix.com \
    --cc=Ian.Campbell@citrix.com \
    --cc=Ian.Jackson@eu.citrix.com \
    --cc=stefano.stabellini@citrix.com \
    --cc=xen-devel@lists.xen.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;
as well as URLs for NNTP newsgroup(s).