From patchwork Sat Nov 10 14:09:25 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Samuel Thibault X-Patchwork-Id: 150739 Delivered-To: patch@linaro.org Received: by 2002:a2e:299d:0:0:0:0:0 with SMTP id p29-v6csp1095640ljp; Sat, 10 Nov 2018 06:22:45 -0800 (PST) X-Google-Smtp-Source: AJdET5cNtedXZqOORYHkVG55J1FwTyEBqko/529/xymnC1ZmUZp37VqLQITaEd5lNCBUfEQJ4Xcr X-Received: by 2002:aed:2fc4:: with SMTP id m62mr11982589qtd.8.1541859765180; Sat, 10 Nov 2018 06:22:45 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1541859765; cv=none; d=google.com; s=arc-20160816; b=BrnBgI7TxwFgFK7RuSDi8S6wFKV3GFCHxF5l6rJ5dIA6Ki47E5DVlliHZdq6sRW69c sGyI6ohYN/Sj9yJ79kAgfJ8lug4BBjh1ym7cnmMc436fxpg9QRMOeFsLZfBAQ3pTn5oE r0OqcDzxsFoSI3T+vEHewDxHnjmR0dkCd56uHK+I+B2EVYanyeTeFZE+Lugs/dL2ymJF z8fnwTH5cqzys/Ksn7LNUZ6m1asbcop5xOteSptc/+52TozelYQe3hdBssKQdjk6WN5v jCYJ4ZWxPwd2PJmVuBILeNSdpfeETvr+n0VngfNeKYP4l1QIB/PgJ7Hojmwk0NDeEo7d 2R9w== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:cc:list-subscribe:list-help:list-post:list-archive :list-unsubscribe:list-id:precedence:subject :content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:to:from; bh=lU8FBnPfgn54fx/SXvnBw59+b0PjGsxfIG9i/3/9bj4=; b=TbPk8uGJeFKeBRHqX13fUCRoc2y0JwDKkBRS1vtLMmW1WxUjk9l++h0zZqtSBfqQUi I/6GoOtZlIQKegiZCVrghemZSVROgjmzR/gxVWOxh4iSegy94flywDy0TDBJOXMqXf+C NJq0QNVwUy/OgcRtjurM5MdY/+75lAq8Lwkbs+zBjzkuNBaSTS4Hwz/qZMBOLhweCXoW aH73B0Y3+9DAGdWL+VSkqROcb8xzMQEVAHmLvl/AT34wbXVjfvxeW4EpACHkQttd0Ikg yL7ChyFNoiIXM+81dBBsrVcX1ty6/Wx7E4ZUSHHgwu+jC8+gmCz9KXdQi1+31sm1ITqx duOw== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of qemu-devel-bounces+patch=linaro.org@nongnu.org designates 2001:4830:134:3::11 as permitted sender) smtp.mailfrom="qemu-devel-bounces+patch=linaro.org@nongnu.org" Return-Path: Received: from lists.gnu.org (lists.gnu.org. [2001:4830:134:3::11]) by mx.google.com with ESMTPS id y3si2490095qvj.17.2018.11.10.06.22.44 for (version=TLS1 cipher=AES128-SHA bits=128/128); Sat, 10 Nov 2018 06:22:45 -0800 (PST) Received-SPF: pass (google.com: domain of qemu-devel-bounces+patch=linaro.org@nongnu.org designates 2001:4830:134:3::11 as permitted sender) client-ip=2001:4830:134:3::11; Authentication-Results: mx.google.com; spf=pass (google.com: domain of qemu-devel-bounces+patch=linaro.org@nongnu.org designates 2001:4830:134:3::11 as permitted sender) smtp.mailfrom="qemu-devel-bounces+patch=linaro.org@nongnu.org" Received: from localhost ([::1]:38639 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gLU9s-00078J-9t for patch@linaro.org; Sat, 10 Nov 2018 09:22:44 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45444) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gLU9B-0006QB-OY for qemu-devel@nongnu.org; Sat, 10 Nov 2018 09:22:02 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gLTxG-0000RA-RF for qemu-devel@nongnu.org; Sat, 10 Nov 2018 09:09:45 -0500 Received: from hera.aquilenet.fr ([2a0c:e300::1]:60400) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1gLTxG-0000H4-Du for qemu-devel@nongnu.org; Sat, 10 Nov 2018 09:09:42 -0500 Received: from localhost (localhost [127.0.0.1]) by hera.aquilenet.fr (Postfix) with ESMTP id 13BF12EC3; Sat, 10 Nov 2018 15:09:30 +0100 (CET) X-Virus-Scanned: Debian amavisd-new at aquilenet.fr Received: from hera.aquilenet.fr ([127.0.0.1]) by localhost (hera.aquilenet.fr [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id AxFRpUVts0ps; Sat, 10 Nov 2018 15:09:29 +0100 (CET) Received: from function (lfbn-bor-1-169-137.w90-50.abo.wanadoo.fr [90.50.25.137]) by hera.aquilenet.fr (Postfix) with ESMTPSA id AE9BC2EC5; Sat, 10 Nov 2018 15:09:26 +0100 (CET) Received: from samy by function with local (Exim 4.91) (envelope-from ) id 1gLTwz-0005r6-QY; Sat, 10 Nov 2018 15:09:25 +0100 From: Samuel Thibault To: qemu-devel@nongnu.org, peter.maydell@linaro.org Date: Sat, 10 Nov 2018 15:09:25 +0100 Message-Id: <20181110140925.22457-5-samuel.thibault@ens-lyon.org> X-Mailer: git-send-email 2.19.1 In-Reply-To: <20181110140925.22457-1-samuel.thibault@ens-lyon.org> References: <20181110140925.22457-1-samuel.thibault@ens-lyon.org> MIME-Version: 1.0 X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 2a0c:e300::1 Subject: [Qemu-devel] [PULL 4/4] slirp: fork_exec(): create and connect child socket before fork() 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: jan.kiszka@siemens.com, stefanha@redhat.com, Samuel Thibault Errors-To: qemu-devel-bounces+patch=linaro.org@nongnu.org Sender: "Qemu-devel" From: Peter Maydell Currently fork_exec() fork()s, and then creates and connects the child socket which it uses for communication with the parent in the child process. This is awkward because the child has no mechanism to report failure back to the parent, which might end up blocked forever in accept(). The child code also has an issue pointed out by Coverity (CID 1005727), where if the qemu_socket() call fails it will pass -1 as a file descriptor to connect(). Fix these issues by moving the creation of the child's end of the socket to before the fork(), where we are in a position to handle a possible failure. Signed-off-by: Peter Maydell Signed-off-by: Samuel Thibault --- slirp/misc.c | 55 ++++++++++++++++++++++++++++++++-------------------- 1 file changed, 34 insertions(+), 21 deletions(-) -- 2.19.1 diff --git a/slirp/misc.c b/slirp/misc.c index 260187b6b6..57bdd808e2 100644 --- a/slirp/misc.c +++ b/slirp/misc.c @@ -85,9 +85,10 @@ fork_exec(struct socket *so, const char *ex, int do_pty) int fork_exec(struct socket *so, const char *ex, int do_pty) { - int s; - struct sockaddr_in addr; + int s, cs; + struct sockaddr_in addr, csaddr; socklen_t addrlen = sizeof(addr); + socklen_t csaddrlen = sizeof(csaddr); int opt; const char *argv[256]; /* don't want to clobber the original */ @@ -120,10 +121,35 @@ fork_exec(struct socket *so, const char *ex, int do_pty) } } + if (getsockname(s, (struct sockaddr *)&csaddr, &csaddrlen) < 0) { + closesocket(s); + return 0; + } + cs = qemu_socket(AF_INET, SOCK_STREAM, 0); + if (cs < 0) { + closesocket(s); + return 0; + } + csaddr.sin_addr = loopback_addr; + /* + * This connect won't block because we've already listen()ed on + * the server end (even though we won't accept() the connection + * until later on). + */ + do { + ret = connect(cs, (struct sockaddr *)&csaddr, csaddrlen); + } while (ret < 0 && errno == EINTR); + if (ret < 0) { + closesocket(s); + closesocket(cs); + return 0; + } + pid = fork(); switch(pid) { case -1: error_report("Error: fork failed: %s", strerror(errno)); + closesocket(cs); close(s); return 0; @@ -131,21 +157,10 @@ fork_exec(struct socket *so, const char *ex, int do_pty) setsid(); /* Set the DISPLAY */ - getsockname(s, (struct sockaddr *)&addr, &addrlen); close(s); - /* - * Connect to the socket - * XXX If any of these fail, we're in trouble! - */ - s = qemu_socket(AF_INET, SOCK_STREAM, 0); - addr.sin_addr = loopback_addr; - do { - ret = connect(s, (struct sockaddr *)&addr, addrlen); - } while (ret < 0 && errno == EINTR); - - dup2(s, 0); - dup2(s, 1); - dup2(s, 2); + dup2(cs, 0); + dup2(cs, 1); + dup2(cs, 2); for (s = getdtablesize() - 1; s >= 3; s--) close(s); @@ -178,12 +193,10 @@ fork_exec(struct socket *so, const char *ex, int do_pty) default: qemu_add_child_watch(pid); + closesocket(cs); /* - * XXX this could block us... - * XXX Should set a timer here, and if accept() doesn't - * return after X seconds, declare it a failure - * The only reason this will block forever is if socket() - * of connect() fail in the child process + * This should never block, because we already connect()ed + * on the child end before we forked. */ do { so->s = accept(s, (struct sockaddr *)&addr, &addrlen);