From patchwork Sat Nov 10 14:09:22 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Samuel Thibault X-Patchwork-Id: 150740 Delivered-To: patch@linaro.org Received: by 2002:a2e:299d:0:0:0:0:0 with SMTP id p29-v6csp1097736ljp; Sat, 10 Nov 2018 06:25:40 -0800 (PST) X-Google-Smtp-Source: AJdET5ehvwYQ6qclqClbaEGOsz7iGROh0WCfWkCjHtP0dTbi57RnjWLvVeX/qTEvBIPvlOtRzilr X-Received: by 2002:aed:25a6:: with SMTP id x35mr12452716qtc.228.1541859940159; Sat, 10 Nov 2018 06:25:40 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1541859940; cv=none; d=google.com; s=arc-20160816; b=gWCoe2zCdMetN90IVA9HcSjZY3nuPaFAd0kkB82NKwQygugUQgZFL+uluymLdQZWwG n48OXlzihgGnkR6Nd/KZUe2uPcgw4bQ8UGz868ib0/W/eVo7uwkFifb3+s8nvqFLoVYO svxwWHtsuOyA2fpu0+dJHLeihib5guBkBuC+TSOVPHkE8sR9DYQRMMegDK/WgElDXap9 qKft7Iwgl/EuYPouP9QFvIn5+mf/+8+vfPuIo0Fq43eJdhuskRzDr1x8DMQIzUbEuyO0 xyoUOdD3v2qNcvMY8HsoA2g3j78VTvi1rZEyKuIcABTQjkMagLzW80nxrjVajbS9iCi5 aS3A== 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=Hr0aXM2/+3n2IT/MiPV2m0oa58McSimfQU4r9t9zAd0=; b=F4i5w1l8VCrSi0PepQnmy0La5VtqtLhoFoPwQ6Ida3BdtYk/9HevxkxX5qfXwumwme Lg6K7aVmgV6SviLlJjuE55tPCSZTzVuq3ayrr/4XfceUxtJ+lDhC0mdscMIfhyurpwJ8 wVt3rsRLK4FPInQNPsy00ky4MQ4f8N/pPn/Xd/2av5BO/Y0zpuxfFnCKfpnKSWxWLD3g zq7fFiRtXDGBRoPww+D40aT7OefxsvhK9rns/MnxJwAbKNXpnKuNCXq0XVqnRiLZnsb5 Nbum2UMgcS3L4IQ/8bGtGI0aMaPOgKwc67V7KnpPlHp7DmBXVcQm3P2tXbzq9qrctgc3 QlDw== 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 h2-v6si2316189qkd.169.2018.11.10.06.25.39 for (version=TLS1 cipher=AES128-SHA bits=128/128); Sat, 10 Nov 2018 06:25:40 -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]:38658 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gLUCh-000370-Hf for patch@linaro.org; Sat, 10 Nov 2018 09:25:39 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45308) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gLU9C-0006GG-2U for qemu-devel@nongnu.org; Sat, 10 Nov 2018 09:22:06 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gLTxG-0000Qw-JE for qemu-devel@nongnu.org; Sat, 10 Nov 2018 09:09:45 -0500 Received: from hera.aquilenet.fr ([185.233.100.1]:44360) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1gLTxG-0000Fp-87 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 8C4F32EC2; Sat, 10 Nov 2018 15:09:28 +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 vJ-2fZAy0MI7; Sat, 10 Nov 2018 15:09:26 +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 67D412EA3; Sat, 10 Nov 2018 15:09:26 +0100 (CET) Received: from samy by function with local (Exim 4.91) (envelope-from ) id 1gLTwz-0005r0-Nu; 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:22 +0100 Message-Id: <20181110140925.22457-2-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: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 185.233.100.1 Subject: [Qemu-devel] [PULL 1/4] slirp: Don't pass possibly -1 fd to send() 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 Coverity complains (CID 1005726) that we might pass -1 as the fd argument to send() in slirp_send(), because we previously checked for "so->s == -1 && so->extra". The case of "so->s == -1 but so->extra NULL" should not in theory happen, but it is hard to guarantee because various places in the code do so->s = qemu_socket(...) and so will end up with so->s == -1 on failure, and not all the paths which call that always throw away the socket in that case (eg tcp_fconnect()). So just check specifically for the condition and fail slirp_send(). Signed-off-by: Peter Maydell Signed-off-by: Samuel Thibault --- slirp/slirp.c | 11 +++++++++++ 1 file changed, 11 insertions(+) -- 2.19.1 diff --git a/slirp/slirp.c b/slirp/slirp.c index 51de41fc02..3c3c03b22f 100644 --- a/slirp/slirp.c +++ b/slirp/slirp.c @@ -1091,6 +1091,17 @@ ssize_t slirp_send(struct socket *so, const void *buf, size_t len, int flags) return len; } + if (so->s == -1) { + /* + * This should in theory not happen but it is hard to be + * sure because some code paths will end up with so->s == -1 + * on a failure but don't dispose of the struct socket. + * Check specifically, so we don't pass -1 to send(). + */ + errno = EBADF; + return -1; + } + return send(so->s, buf, len, flags); } From patchwork Sat Nov 10 14:09:23 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Samuel Thibault X-Patchwork-Id: 150741 Delivered-To: patch@linaro.org Received: by 2002:a2e:299d:0:0:0:0:0 with SMTP id p29-v6csp1099015ljp; Sat, 10 Nov 2018 06:27:19 -0800 (PST) X-Google-Smtp-Source: AJdET5eLTNF7J6mazt4ED9KNWMQRhMF4HcAvvnZRCbfdFYlMxcBfITxra9yEBkNhTVP68cP6fmve X-Received: by 2002:ac8:3715:: with SMTP id o21mr9053527qtb.224.1541860039880; Sat, 10 Nov 2018 06:27:19 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1541860039; cv=none; d=google.com; s=arc-20160816; b=J0HTgN+6Hm3CiXtMscSjJ0/ZPGyCj6Fwlp/1u4Gxj6TpbSgU50F/PTL9vNNdegDvRo f4fyLsu/S2kHV40swBkqsMbWJxOEKSf9NWxAIyDpeg6IMnHikFNR76suzryDUqGwo5rA rrr0xc26E1/RQPepOFKEZqZJq9NMR+WTqbLcH9W5X5mlSlR8Xy7KtOERBBa7/nQx1/Ij VnKh8dTuyfXtg88iPUMFBdeQj3cDRbJQZMmkQAc2oDBKpcXxydxTdeebTU5ezbqKQvm4 snxeDh8RBsPzTdCY+wg6B8SMp5DIHtrp/uBVfzbSCBAXjYEJk6MK2H1d4yOVPXcOVUMc jbVA== 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=1FzX9raC+L2y2SP7W2zAAEUwaQ91MYO8uS4G120kWFA=; b=0aFmNzKqUTNCYCRpndpfZ5fyJOgr1lAyfuLqiaPB1kq/41sNL0GasDoPoqbT12jR08 ux3mYwqVC0pg/F0RKIL4erQRbtPV8D1jZnjVduaHfWzIb2cWiBOSmZn8hzUpowQIe3eg UY3kXYuOO8ZUjmTD5GWh6bEH3pCyABrAjqbeWilphrDncipSOtDgQnS+/Ifvl6PUPyFj H9MMxqbIYxwohLa3zrJ/zYPZkHceuYZbbBRWXVvTX1O96BM70j9Cs6X7RyskFBtWG6QE kUio++8SsgAuCeXDR3R9i+q7xkK08kp1YarM1CQ1VFQy4A4S8MUB+YKJqVyb21BisXlU M8Fw== 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 k34si4946939qtd.156.2018.11.10.06.27.19 for (version=TLS1 cipher=AES128-SHA bits=128/128); Sat, 10 Nov 2018 06:27:19 -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]:38667 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gLUEJ-0004cf-Ad for patch@linaro.org; Sat, 10 Nov 2018 09:27:19 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45792) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gLU9B-0006rS-Ui for qemu-devel@nongnu.org; Sat, 10 Nov 2018 09:22:06 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gLTxG-0000Qn-FP for qemu-devel@nongnu.org; Sat, 10 Nov 2018 09:09:45 -0500 Received: from hera.aquilenet.fr ([185.233.100.1]:44348) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1gLTxG-0000F7-5s 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 2F2BC1218; Sat, 10 Nov 2018 15:09:28 +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 mqXSEfKLFVhQ; Sat, 10 Nov 2018 15:09:27 +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 901502EC2; Sat, 10 Nov 2018 15:09:26 +0100 (CET) Received: from samy by function with local (Exim 4.91) (envelope-from ) id 1gLTwz-0005r2-Oi; 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:23 +0100 Message-Id: <20181110140925.22457-3-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: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 185.233.100.1 Subject: [Qemu-devel] [PULL 2/4] slirp: Use g_new() to allocate sockets in socreate() 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 The slirp socreate() function can only fail if the attempt to malloc() the struct socket fails. Switch to using g_new() instead, which will allow us to remove the error-handling code from its callers. Signed-off-by: Peter Maydell Signed-off-by: Samuel Thibault --- slirp/socket.c | 14 ++++++-------- slirp/tcp_input.c | 4 ++-- slirp/tcp_subr.c | 2 +- 3 files changed, 9 insertions(+), 11 deletions(-) -- 2.19.1 diff --git a/slirp/socket.c b/slirp/socket.c index 322383a1f9..35a9a14565 100644 --- a/slirp/socket.c +++ b/slirp/socket.c @@ -46,17 +46,15 @@ struct socket *solookup(struct socket **last, struct socket *head, struct socket * socreate(Slirp *slirp) { - struct socket *so; + struct socket *so = g_new(struct socket, 1); - so = (struct socket *)malloc(sizeof(struct socket)); - if(so) { memset(so, 0, sizeof(struct socket)); so->so_state = SS_NOFDREF; so->s = -1; so->slirp = slirp; so->pollfds_idx = -1; - } - return(so); + + return so; } /* @@ -110,7 +108,7 @@ sofree(struct socket *so) if (so->so_tcpcb) { free(so->so_tcpcb); } - free(so); + g_free(so); } size_t sopreprbuf(struct socket *so, struct iovec *iov, int *np) @@ -721,8 +719,8 @@ tcp_listen(Slirp *slirp, uint32_t haddr, u_int hport, uint32_t laddr, /* Don't tcp_attach... we don't need so_snd nor so_rcv */ if ((so->so_tcpcb = tcp_newtcpcb(so)) == NULL) { - free(so); - return NULL; + g_free(so); + return NULL; } insque(so, &slirp->tcb); diff --git a/slirp/tcp_input.c b/slirp/tcp_input.c index 07bcbdb2dd..4f79c95fdb 100644 --- a/slirp/tcp_input.c +++ b/slirp/tcp_input.c @@ -432,8 +432,8 @@ findso: if ((so = socreate(slirp)) == NULL) goto dropwithreset; if (tcp_attach(so) < 0) { - free(so); /* Not sofree (if it failed, it's not insqued) */ - goto dropwithreset; + g_free(so); /* Not sofree (if it failed, it's not insqued) */ + goto dropwithreset; } sbreserve(&so->so_snd, TCP_SNDSPACE); diff --git a/slirp/tcp_subr.c b/slirp/tcp_subr.c index 8d0f94b75f..0270c89ae3 100644 --- a/slirp/tcp_subr.c +++ b/slirp/tcp_subr.c @@ -475,7 +475,7 @@ void tcp_connect(struct socket *inso) return; } if (tcp_attach(so) < 0) { - free(so); /* NOT sofree */ + g_free(so); /* NOT sofree */ return; } so->lhost = inso->lhost; From patchwork Sat Nov 10 14:09:24 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Samuel Thibault X-Patchwork-Id: 150738 Delivered-To: patch@linaro.org Received: by 2002:a2e:299d:0:0:0:0:0 with SMTP id p29-v6csp1095460ljp; Sat, 10 Nov 2018 06:22:32 -0800 (PST) X-Google-Smtp-Source: AJdET5fTxU/4gyW4flflS6vME+FiNE4KCj/hvVXDyW5uCu0jsyEEeZVSxQGjkSOStJwf8sOsCQRg X-Received: by 2002:aed:26e1:: with SMTP id q88mr12627917qtd.148.1541859752732; Sat, 10 Nov 2018 06:22:32 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1541859752; cv=none; d=google.com; s=arc-20160816; b=yuGHS4kVHbyFpinABG2LPSEIuejS7Y74/pffBEk6eyCnuLxpTJVimrdI5jPcrbo47w Q/pID5Am7NfMVMTjsVAZLNnS2qmUXbyYkB+7HDiozkFl0WWKq7ze2sISYY5NN6ZPtMoI NO30QNxXwZ73JreXH+WcDgeDYIFqfOhFkhcPPjiwPhivvZUGeHN1vI7duPPupDEQjx9d QOpCLWtLj1rafunrp0SSu9yhTcms6b4lW0RgrwrlHw1+CmPOgXhhiXk0B6LyVPgRLFJv 2WKF0YnchUeXGFc3xd8HR06u+rB4r9LlRPlJkc9FKqct82YEcQftMvSp/q/eg3KxFKUh hunQ== 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=/R+co+fpgiWDXAALpa864iHTU4sOjlqnzrh8rprT5w8=; b=Rwh3mj8TSbu/C6ueCesidsIM7W692sYqJUPZmUAxk7kQ8b1R7ffKWwgkNZiu5DVYGM 3EQjD1dz+e4JvjJ5IwkLna6E1LVdX9gGDRNcJOk3upTjb/1o/DnntA0PSDarGTs42QAI jGGkMBf86f2oLiEIKWHPDhOK2QRtXoYNFxJs0mZ9FgYAxOou/48iTUlfw2kHwmPw3kwH uhFbKq1kJiyR/4sceNI15L0r9U35jeM1javgOfcNKe9IU/Dd7p1LodYQyDx2sRWTiVt9 ZhSAhThgyasWOoKlls4c/ZZFfi1/qkDOxUcFfUVgGJNHfVnExakmLnUx9U/VEfRCY7+N D2PA== 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 i21si4831907qtp.305.2018.11.10.06.22.32 for (version=TLS1 cipher=AES128-SHA bits=128/128); Sat, 10 Nov 2018 06:22:32 -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]:38632 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gLU9g-0006wz-15 for patch@linaro.org; Sat, 10 Nov 2018 09:22:32 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45792) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gLU97-0006rS-GE for qemu-devel@nongnu.org; Sat, 10 Nov 2018 09:22:01 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gLTxH-0000Ra-CN for qemu-devel@nongnu.org; Sat, 10 Nov 2018 09:09:45 -0500 Received: from hera.aquilenet.fr ([185.233.100.1]:44370) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1gLTxH-0000Gm-0x for qemu-devel@nongnu.org; Sat, 10 Nov 2018 09:09:43 -0500 Received: from localhost (localhost [127.0.0.1]) by hera.aquilenet.fr (Postfix) with ESMTP id 7F7272EA3; Sat, 10 Nov 2018 15:09:29 +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 aTvM1MTqIb0x; Sat, 10 Nov 2018 15:09:28 +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 A02792EC4; Sat, 10 Nov 2018 15:09:26 +0100 (CET) Received: from samy by function with local (Exim 4.91) (envelope-from ) id 1gLTwz-0005r4-Pb; 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:24 +0100 Message-Id: <20181110140925.22457-4-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: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 185.233.100.1 Subject: [Qemu-devel] [PULL 3/4] slirp: Remove code that handles socreate() failure 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 Now that socreate() can never fail, we can remove the code that was trying to handle that situation. In particular this removes code in tcp_connect() that provoked Coverity to complain (CID 1005724): in closesocket(accept(inso->s, (struct sockaddr *)&addr, &addrlen)); if the accept() call fails then we pass closesocket() -1 instead of a valid file descriptor. Signed-off-by: Peter Maydell Signed-off-by: Samuel Thibault --- slirp/ip_icmp.c | 2 +- slirp/slirp.c | 3 --- slirp/socket.c | 3 --- slirp/tcp_input.c | 3 +-- slirp/tcp_subr.c | 5 ----- slirp/udp.c | 6 ------ slirp/udp6.c | 3 --- 7 files changed, 2 insertions(+), 23 deletions(-) -- 2.19.1 diff --git a/slirp/ip_icmp.c b/slirp/ip_icmp.c index da100d1f55..9210eef3f3 100644 --- a/slirp/ip_icmp.c +++ b/slirp/ip_icmp.c @@ -160,7 +160,7 @@ icmp_input(struct mbuf *m, int hlen) } else { struct socket *so; struct sockaddr_storage addr; - if ((so = socreate(slirp)) == NULL) goto freeit; + so = socreate(slirp); if (icmp_send(so, m, hlen) == 0) { return; } diff --git a/slirp/slirp.c b/slirp/slirp.c index 3c3c03b22f..322edf51eb 100644 --- a/slirp/slirp.c +++ b/slirp/slirp.c @@ -1477,9 +1477,6 @@ static int slirp_state_load(QEMUFile *f, void *opaque, int version_id) int ret; struct socket *so = socreate(slirp); - if (!so) - return -ENOMEM; - ret = vmstate_load_state(f, &vmstate_slirp_socket, so, version_id); if (ret < 0) diff --git a/slirp/socket.c b/slirp/socket.c index 35a9a14565..c01d8696af 100644 --- a/slirp/socket.c +++ b/slirp/socket.c @@ -713,9 +713,6 @@ tcp_listen(Slirp *slirp, uint32_t haddr, u_int hport, uint32_t laddr, DEBUG_ARG("flags = %x", flags); so = socreate(slirp); - if (!so) { - return NULL; - } /* Don't tcp_attach... we don't need so_snd nor so_rcv */ if ((so->so_tcpcb = tcp_newtcpcb(so)) == NULL) { diff --git a/slirp/tcp_input.c b/slirp/tcp_input.c index 4f79c95fdb..d073ef9525 100644 --- a/slirp/tcp_input.c +++ b/slirp/tcp_input.c @@ -429,8 +429,7 @@ findso: if ((tiflags & (TH_SYN|TH_FIN|TH_RST|TH_URG|TH_ACK)) != TH_SYN) goto dropwithreset; - if ((so = socreate(slirp)) == NULL) - goto dropwithreset; + so = socreate(slirp); if (tcp_attach(so) < 0) { g_free(so); /* Not sofree (if it failed, it's not insqued) */ goto dropwithreset; diff --git a/slirp/tcp_subr.c b/slirp/tcp_subr.c index 0270c89ae3..fa61349cbb 100644 --- a/slirp/tcp_subr.c +++ b/slirp/tcp_subr.c @@ -469,11 +469,6 @@ void tcp_connect(struct socket *inso) so = inso; } else { so = socreate(slirp); - if (so == NULL) { - /* If it failed, get rid of the pending connection */ - closesocket(accept(inso->s, (struct sockaddr *)&addr, &addrlen)); - return; - } if (tcp_attach(so) < 0) { g_free(so); /* NOT sofree */ return; diff --git a/slirp/udp.c b/slirp/udp.c index e5bf065bf2..c47870a61b 100644 --- a/slirp/udp.c +++ b/slirp/udp.c @@ -171,9 +171,6 @@ udp_input(register struct mbuf *m, int iphlen) * create one */ so = socreate(slirp); - if (!so) { - goto bad; - } if (udp_attach(so, AF_INET) == -1) { DEBUG_MISC((dfd," udp_attach errno = %d-%s\n", errno,strerror(errno))); @@ -331,9 +328,6 @@ udp_listen(Slirp *slirp, uint32_t haddr, u_int hport, uint32_t laddr, socklen_t addrlen = sizeof(struct sockaddr_in); so = socreate(slirp); - if (!so) { - return NULL; - } so->s = qemu_socket(AF_INET,SOCK_DGRAM,0); if (so->s < 0) { sofree(so); diff --git a/slirp/udp6.c b/slirp/udp6.c index 7c4a6b003a..986010f0d3 100644 --- a/slirp/udp6.c +++ b/slirp/udp6.c @@ -91,9 +91,6 @@ void udp6_input(struct mbuf *m) if (so == NULL) { /* If there's no socket for this packet, create one. */ so = socreate(slirp); - if (!so) { - goto bad; - } if (udp_attach(so, AF_INET6) == -1) { DEBUG_MISC((dfd, " udp6_attach errno = %d-%s\n", errno, strerror(errno))); 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);