{"id":810052,"url":"http://patchwork.ozlabs.org/api/patches/810052/?format=json","web_url":"http://patchwork.ozlabs.org/project/qemu-devel/patch/20170905092230.8243-6-berrange@redhat.com/","project":{"id":14,"url":"http://patchwork.ozlabs.org/api/projects/14/?format=json","name":"QEMU Development","link_name":"qemu-devel","list_id":"qemu-devel.nongnu.org","list_email":"qemu-devel@nongnu.org","web_url":"","scm_url":"","webscm_url":"","list_archive_url":"","list_archive_url_format":"","commit_url_format":""},"msgid":"<20170905092230.8243-6-berrange@redhat.com>","list_archive_url":null,"date":"2017-09-05T09:22:26","name":"[PULL,v1,5/9] sockets: Handle race condition between binds to the same port","commit_ref":null,"pull_url":null,"state":"new","archived":false,"hash":"7a083eac8508a0edb33d2c1084e4526364ef9b7e","submitter":{"id":2694,"url":"http://patchwork.ozlabs.org/api/people/2694/?format=json","name":"Daniel P. Berrangé","email":"berrange@redhat.com"},"delegate":null,"mbox":"http://patchwork.ozlabs.org/project/qemu-devel/patch/20170905092230.8243-6-berrange@redhat.com/mbox/","series":[{"id":1527,"url":"http://patchwork.ozlabs.org/api/series/1527/?format=json","web_url":"http://patchwork.ozlabs.org/project/qemu-devel/list/?series=1527","date":"2017-09-05T09:22:21","name":"[PULL,v1,1/9] io: fix temp directory used by test-io-channel-tls test","version":1,"mbox":"http://patchwork.ozlabs.org/series/1527/mbox/"}],"comments":"http://patchwork.ozlabs.org/api/patches/810052/comments/","check":"pending","checks":"http://patchwork.ozlabs.org/api/patches/810052/checks/","tags":{},"related":[],"headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=)","ext-mx04.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx04.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=berrange@redhat.com"],"Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xmhKT4GZXz9s0g\n\tfor ;\n\tTue, 5 Sep 2017 19:30:53 +1000 (AEST)","from localhost ([::1]:57700 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1dpAC3-00082O-Ig\n\tfor incoming@patchwork.ozlabs.org; Tue, 05 Sep 2017 05:30:51 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:41189)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1dpA4F-0001Bi-3m\n\tfor qemu-devel@nongnu.org; Tue, 05 Sep 2017 05:22:56 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1dpA49-0002jd-RV\n\tfor qemu-devel@nongnu.org; Tue, 05 Sep 2017 05:22:47 -0400","from mx1.redhat.com ([209.132.183.28]:36348)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from ) id 1dpA49-0002ia-JS\n\tfor qemu-devel@nongnu.org; Tue, 05 Sep 2017 05:22:41 -0400","from smtp.corp.redhat.com\n\t(int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id B3A907EA8B;\n\tTue, 5 Sep 2017 09:22:40 +0000 (UTC)","from t460.redhat.com (unknown [10.33.36.63])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id A79D560BEC;\n\tTue, 5 Sep 2017 09:22:39 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com B3A907EA8B","From":"\"Daniel P. Berrange\" ","To":"qemu-devel@nongnu.org","Date":"Tue, 5 Sep 2017 10:22:26 +0100","Message-Id":"<20170905092230.8243-6-berrange@redhat.com>","In-Reply-To":"<20170905092230.8243-1-berrange@redhat.com>","References":"<20170905092230.8243-1-berrange@redhat.com>","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.13","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.28]);\n\tTue, 05 Sep 2017 09:22:40 +0000 (UTC)","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]\n\t[fuzzy]","X-Received-From":"209.132.183.28","Subject":"[Qemu-devel] [PULL v1 5/9] sockets: Handle race condition between\n\tbinds to the same port","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"Peter Maydell ,\n\tKnut Omang ","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"},"content":"From: Knut Omang \n\nIf an offset of ports is specified to the inet_listen_saddr function(),\nand two or more processes tries to bind from these ports at the same time,\noccasionally more than one process may be able to bind to the same\nport. The condition is detected by listen() but too late to avoid a failure.\n\nThis function is called by socket_listen() and used\nby all socket listening code in QEMU, so all cases where any form of dynamic\nport selection is used should be subject to this issue.\n\nAdd code to close and re-establish the socket when this\ncondition is observed, hiding the race condition from the user.\n\nAlso clean up some issues with error handling to allow more\naccurate reporting of the cause of an error.\n\nThis has been developed and tested by means of the\ntest-listen unit test in the previous commit.\nEnable the test for make check now that it passes.\n\nReviewed-by: Bhavesh Davda \nReviewed-by: Yuval Shaia \nReviewed-by: Girish Moodalbail \nSigned-off-by: Knut Omang \nReviewed-by: Daniel P. Berrange \nSigned-off-by: Daniel P. Berrange \n---\n tests/Makefile.include | 2 +-\n util/qemu-sockets.c | 58 +++++++++++++++++++++++++++++++++++---------------\n 2 files changed, 42 insertions(+), 18 deletions(-)","diff":"diff --git a/tests/Makefile.include b/tests/Makefile.include\nindex a231754517..eef4088d5c 100644\n--- a/tests/Makefile.include\n+++ b/tests/Makefile.include\n@@ -151,7 +151,7 @@ check-unit-y += tests/test-bufferiszero$(EXESUF)\n gcov-files-check-bufferiszero-y = util/bufferiszero.c\n check-unit-y += tests/test-uuid$(EXESUF)\n check-unit-y += tests/ptimer-test$(EXESUF)\n-#check-unit-y += tests/test-listen$(EXESUF)\n+check-unit-y += tests/test-listen$(EXESUF)\n gcov-files-ptimer-test-y = hw/core/ptimer.c\n check-unit-y += tests/test-qapi-util$(EXESUF)\n gcov-files-test-qapi-util-y = qapi/qapi-util.c\ndiff --git a/util/qemu-sockets.c b/util/qemu-sockets.c\nindex d0d20476e5..6a511fbf76 100644\n--- a/util/qemu-sockets.c\n+++ b/util/qemu-sockets.c\n@@ -206,7 +206,10 @@ static int inet_listen_saddr(InetSocketAddress *saddr,\n char port[33];\n char uaddr[INET6_ADDRSTRLEN+1];\n char uport[33];\n- int slisten, rc, port_min, port_max, p;\n+ int rc, port_min, port_max, p;\n+ int slisten = 0;\n+ int saved_errno = 0;\n+ bool socket_created = false;\n Error *err = NULL;\n \n memset(&ai,0, sizeof(ai));\n@@ -258,7 +261,7 @@ static int inet_listen_saddr(InetSocketAddress *saddr,\n return -1;\n }\n \n- /* create socket + bind */\n+ /* create socket + bind/listen */\n for (e = res; e != NULL; e = e->ai_next) {\n getnameinfo((struct sockaddr*)e->ai_addr,e->ai_addrlen,\n \t\t uaddr,INET6_ADDRSTRLEN,uport,32,\n@@ -266,37 +269,58 @@ static int inet_listen_saddr(InetSocketAddress *saddr,\n \n slisten = create_fast_reuse_socket(e);\n if (slisten < 0) {\n- if (!e->ai_next) {\n- error_setg_errno(errp, errno, \"Failed to create socket\");\n- }\n continue;\n }\n \n+ socket_created = true;\n port_min = inet_getport(e);\n port_max = saddr->has_to ? saddr->to + port_offset : port_min;\n for (p = port_min; p <= port_max; p++) {\n inet_setport(e, p);\n- if (try_bind(slisten, saddr, e) >= 0) {\n- goto listen;\n- }\n- if (p == port_max) {\n- if (!e->ai_next) {\n+ rc = try_bind(slisten, saddr, e);\n+ if (rc) {\n+ if (errno == EADDRINUSE) {\n+ continue;\n+ } else {\n error_setg_errno(errp, errno, \"Failed to bind socket\");\n+ goto listen_failed;\n }\n }\n+ if (!listen(slisten, 1)) {\n+ goto listen_ok;\n+ }\n+ if (errno != EADDRINUSE) {\n+ error_setg_errno(errp, errno, \"Failed to listen on socket\");\n+ goto listen_failed;\n+ }\n+ /* Someone else managed to bind to the same port and beat us\n+ * to listen on it! Socket semantics does not allow us to\n+ * recover from this situation, so we need to recreate the\n+ * socket to allow bind attempts for subsequent ports:\n+ */\n+ closesocket(slisten);\n+ slisten = create_fast_reuse_socket(e);\n+ if (slisten < 0) {\n+ error_setg_errno(errp, errno,\n+ \"Failed to recreate failed listening socket\");\n+ goto listen_failed;\n+ }\n }\n+ }\n+ error_setg_errno(errp, errno,\n+ socket_created ?\n+ \"Failed to find an available port\" :\n+ \"Failed to create a socket\");\n+listen_failed:\n+ saved_errno = errno;\n+ if (slisten >= 0) {\n closesocket(slisten);\n }\n freeaddrinfo(res);\n+ errno = saved_errno;\n return -1;\n \n-listen:\n- if (listen(slisten,1) != 0) {\n- error_setg_errno(errp, errno, \"Failed to listen on socket\");\n- closesocket(slisten);\n- freeaddrinfo(res);\n- return -1;\n- }\n+listen_ok:\n if (update_addr) {\n g_free(saddr->host);\n saddr->host = g_strdup(uaddr);\n","prefixes":["PULL","v1","5/9"]}