From patchwork Sat Jul 29 21:18:18 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Knut Omang X-Patchwork-Id: 9870165 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 7E9F860365 for ; Sat, 29 Jul 2017 21:21:29 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 7266828555 for ; Sat, 29 Jul 2017 21:21:29 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 6717C285CA; Sat, 29 Jul 2017 21:21:29 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, UNPARSEABLE_RELAY autolearn=ham version=3.3.1 Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id B94F528555 for ; Sat, 29 Jul 2017 21:21:28 +0000 (UTC) Received: from localhost ([::1]:53458 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dbZAu-0006DR-1o for patchwork-qemu-devel@patchwork.kernel.org; Sat, 29 Jul 2017 17:21:28 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:32824) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dbZ8E-0003bq-Er for qemu-devel@nongnu.org; Sat, 29 Jul 2017 17:18:43 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dbZ8A-0001Rh-1w for qemu-devel@nongnu.org; Sat, 29 Jul 2017 17:18:42 -0400 Received: from aserp1040.oracle.com ([141.146.126.69]:25607) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1dbZ89-0001P2-MZ for qemu-devel@nongnu.org; Sat, 29 Jul 2017 17:18:37 -0400 Received: from aserv0021.oracle.com (aserv0021.oracle.com [141.146.126.233]) by aserp1040.oracle.com (Sentrion-MTA-4.3.2/Sentrion-MTA-4.3.2) with ESMTP id v6TLIaMp016329 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Sat, 29 Jul 2017 21:18:36 GMT Received: from userv0122.oracle.com (userv0122.oracle.com [156.151.31.75]) by aserv0021.oracle.com (8.14.4/8.14.4) with ESMTP id v6TLIZUb000764 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Sat, 29 Jul 2017 21:18:36 GMT Received: from abhmp0016.oracle.com (abhmp0016.oracle.com [141.146.116.22]) by userv0122.oracle.com (8.14.4/8.14.4) with ESMTP id v6TLIZOY010710; Sat, 29 Jul 2017 21:18:35 GMT Received: from abi.no.oracle.com (/10.172.144.123) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Sat, 29 Jul 2017 14:18:35 -0700 From: Knut Omang To: "Daniel P . Berrange" , Gerd Hoffmann , Paolo Bonzini Date: Sat, 29 Jul 2017 23:18:18 +0200 Message-Id: <76cb37b3decd6d6c1c9bac72e21a06fabf7579be.1501362828.git-series.knut.omang@oracle.com> X-Mailer: git-send-email 2.9.4 In-Reply-To: References: X-Source-IP: aserv0021.oracle.com [141.146.126.233] X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.4.x-2.6.x [generic] [fuzzy] X-Received-From: 141.146.126.69 Subject: [Qemu-devel] [PATCH v6 4/4] sockets: Handle race condition between binds to the same port X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Knut Omang , qemu-devel@nongnu.org Errors-To: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: "Qemu-devel" X-Virus-Scanned: ClamAV using ClamSMTP If an offset of ports is specified to the inet_listen_saddr function(), and two or more processes tries to bind from these ports at the same time, occasionally more than one process may be able to bind to the same port. The condition is detected by listen() but too late to avoid a failure. This function is called by socket_listen() and used by all socket listening code in QEMU, so all cases where any form of dynamic port selection is used should be subject to this issue. Add code to close and re-establish the socket when this condition is observed, hiding the race condition from the user. Also clean up some issues with error handling to allow more accurate reporting of the cause of an error. This has been developed and tested by means of the test-listen unit test in the previous commit. Enable the test for make check now that it passes. Reviewed-by: Bhavesh Davda Reviewed-by: Yuval Shaia Reviewed-by: Girish Moodalbail Signed-off-by: Knut Omang Reviewed-by: Daniel P. Berrange --- tests/Makefile.include | 2 +- util/qemu-sockets.c | 58 ++++++++++++++++++++++++++++++------------- 2 files changed, 42 insertions(+), 18 deletions(-) diff --git a/tests/Makefile.include b/tests/Makefile.include index b37c0c8..9d2131d 100644 --- a/tests/Makefile.include +++ b/tests/Makefile.include @@ -128,7 +128,7 @@ check-unit-y += tests/test-bufferiszero$(EXESUF) gcov-files-check-bufferiszero-y = util/bufferiszero.c check-unit-y += tests/test-uuid$(EXESUF) check-unit-y += tests/ptimer-test$(EXESUF) -#check-unit-y += tests/test-listen$(EXESUF) +check-unit-y += tests/test-listen$(EXESUF) gcov-files-ptimer-test-y = hw/core/ptimer.c check-unit-y += tests/test-qapi-util$(EXESUF) gcov-files-test-qapi-util-y = qapi/qapi-util.c diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c index d0d2047..6a511fb 100644 --- a/util/qemu-sockets.c +++ b/util/qemu-sockets.c @@ -206,7 +206,10 @@ static int inet_listen_saddr(InetSocketAddress *saddr, char port[33]; char uaddr[INET6_ADDRSTRLEN+1]; char uport[33]; - int slisten, rc, port_min, port_max, p; + int rc, port_min, port_max, p; + int slisten = 0; + int saved_errno = 0; + bool socket_created = false; Error *err = NULL; memset(&ai,0, sizeof(ai)); @@ -258,7 +261,7 @@ static int inet_listen_saddr(InetSocketAddress *saddr, return -1; } - /* create socket + bind */ + /* create socket + bind/listen */ for (e = res; e != NULL; e = e->ai_next) { getnameinfo((struct sockaddr*)e->ai_addr,e->ai_addrlen, uaddr,INET6_ADDRSTRLEN,uport,32, @@ -266,37 +269,58 @@ static int inet_listen_saddr(InetSocketAddress *saddr, slisten = create_fast_reuse_socket(e); if (slisten < 0) { - if (!e->ai_next) { - error_setg_errno(errp, errno, "Failed to create socket"); - } continue; } + socket_created = true; port_min = inet_getport(e); port_max = saddr->has_to ? saddr->to + port_offset : port_min; for (p = port_min; p <= port_max; p++) { inet_setport(e, p); - if (try_bind(slisten, saddr, e) >= 0) { - goto listen; - } - if (p == port_max) { - if (!e->ai_next) { + rc = try_bind(slisten, saddr, e); + if (rc) { + if (errno == EADDRINUSE) { + continue; + } else { error_setg_errno(errp, errno, "Failed to bind socket"); + goto listen_failed; } } + if (!listen(slisten, 1)) { + goto listen_ok; + } + if (errno != EADDRINUSE) { + error_setg_errno(errp, errno, "Failed to listen on socket"); + goto listen_failed; + } + /* Someone else managed to bind to the same port and beat us + * to listen on it! Socket semantics does not allow us to + * recover from this situation, so we need to recreate the + * socket to allow bind attempts for subsequent ports: + */ + closesocket(slisten); + slisten = create_fast_reuse_socket(e); + if (slisten < 0) { + error_setg_errno(errp, errno, + "Failed to recreate failed listening socket"); + goto listen_failed; + } } + } + error_setg_errno(errp, errno, + socket_created ? + "Failed to find an available port" : + "Failed to create a socket"); +listen_failed: + saved_errno = errno; + if (slisten >= 0) { closesocket(slisten); } freeaddrinfo(res); + errno = saved_errno; return -1; -listen: - if (listen(slisten,1) != 0) { - error_setg_errno(errp, errno, "Failed to listen on socket"); - closesocket(slisten); - freeaddrinfo(res); - return -1; - } +listen_ok: if (update_addr) { g_free(saddr->host); saddr->host = g_strdup(uaddr);