From nobody Fri Apr 11 15:57:19 2025 Delivered-To: importer@patchew.org Authentication-Results: mx.zohomail.com; dkim=fail; spf=pass (zohomail.com: domain of gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail(p=none dis=none) header.from=redhat.com ARC-Seal: i=1; a=rsa-sha256; t=1594821368; cv=none; d=zohomail.com; s=zohoarc; b=E5cb5h4Lm2Dii3fvhuHc5k2hvvHWa7icp02FWZzCyofo+GY0uHuHGhxqIUpTo+WsnZaVLXQet990fqb1GPod8eAN1uPphastLN0UWga3KMncozzIga84bZ9ZAjp/jt4aCEW2Latys3OnKDmvft9K6ppkt67rhG2gOmc38oKU46s= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1594821368; h=Content-Type:Content-Transfer-Encoding:Cc:Date:From:In-Reply-To:List-Subscribe:List-Post:List-Id:List-Archive:List-Help:List-Unsubscribe:MIME-Version:Message-ID:References:Sender:Subject:To; bh=rcS9JhqiNRxIxolYBZdtW2tdwI0vimd9/hYJ9NxOy5Y=; b=HMhluY7OlPGTbnf1HRRgrvRpjfUr5pF4lNt+enaxNWRo7elhSBa98nzsupYLiRyInggYCCRC636maB49QgHGCL6engZCagVQCM9hV4wgf+kq9DDDcmT0dT1Fri4H58qwZhWfRL8WvMWuzP8rXKHE7CAcLLCQ5iu13KagnEmX6BQ= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=fail; spf=pass (zohomail.com: domain of gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail header.from= (p=none dis=none) header.from= Return-Path: Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) by mx.zohomail.com with SMTPS id 15948213689361005.1953354163101; Wed, 15 Jul 2020 06:56:08 -0700 (PDT) Received: from localhost ([::1]:37782 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1jvhtH-0007U3-GQ for importer@patchew.org; Wed, 15 Jul 2020 09:56:07 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:40636) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1jvhr3-00039X-AZ for qemu-devel@nongnu.org; Wed, 15 Jul 2020 09:53:49 -0400 Received: from us-smtp-delivery-1.mimecast.com ([207.211.31.120]:39261 helo=us-smtp-1.mimecast.com) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.90_1) (envelope-from ) id 1jvhr0-0005ML-V0 for qemu-devel@nongnu.org; Wed, 15 Jul 2020 09:53:49 -0400 Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-435-SvgJtXYuMN6ct6jxnwb70w-1; Wed, 15 Jul 2020 09:53:44 -0400 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 42878E91A; Wed, 15 Jul 2020 13:53:43 +0000 (UTC) Received: from jason-ThinkPad-T430s.redhat.com (ovpn-12-28.pek2.redhat.com [10.72.12.28]) by smtp.corp.redhat.com (Postfix) with ESMTP id 9E36D1755E; Wed, 15 Jul 2020 13:53:41 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1594821226; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=rcS9JhqiNRxIxolYBZdtW2tdwI0vimd9/hYJ9NxOy5Y=; b=XVKoCYeYOuw8yZpdyM8bGXRcd7FFR9FbShiSGkO6qmTuvHlJDeseWs3OwPfrt32rx43MXJ bYDURDvT5XQcWZJ2Yszog8OgZoqR53zXec3Yw+iRpztLezE4Xsjf1uYwCHnx7HlhMrZh3J hoTSCviEoQC59M0RH1g7atQR9AkJS9U= X-MC-Unique: SvgJtXYuMN6ct6jxnwb70w-1 From: Jason Wang To: peter.maydell@linaro.org Subject: [PULL 5/7] net: check if the file descriptor is valid before using it Date: Wed, 15 Jul 2020 21:53:19 +0800 Message-Id: <1594821201-3708-6-git-send-email-jasowang@redhat.com> In-Reply-To: <1594821201-3708-1-git-send-email-jasowang@redhat.com> References: <1594821201-3708-1-git-send-email-jasowang@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Received-SPF: pass (zohomail.com: domain of gnu.org designates 209.51.188.17 as permitted sender) client-ip=209.51.188.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Received-SPF: pass client-ip=207.211.31.120; envelope-from=jasowang@redhat.com; helo=us-smtp-1.mimecast.com X-detected-operating-system: by eggs.gnu.org: First seen = 2020/07/15 05:07:28 X-ACL-Warn: Detected OS = Linux 2.2.x-3.x [generic] [fuzzy] X-Spam_score_int: -40 X-Spam_score: -4.1 X-Spam_bar: ---- X-Spam_report: (-4.1 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-1, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H2=-1, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Laurent Vivier , Jason Wang , qemu-devel@nongnu.org Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail-DKIM: fail (Header signature does not verify) From: Laurent Vivier qemu_set_nonblock() checks that the file descriptor can be used and, if not, crashes QEMU. An assert() is used for that. The use of assert() is used to detect programming error and the coredump will allow to debug the problem. But in the case of the tap device, this assert() can be triggered by a misconfiguration by the user. At startup, it's not a real problem, but it can also happen during the hot-plug of a new device, and here it's a problem because we can crash a perfectly healthy system. For instance: # ip link add link virbr0 name macvtap0 type macvtap mode bridge # ip link set macvtap0 up # TAP=3D/dev/tap$(ip -o link show macvtap0 | cut -d: -f1) # qemu-system-x86_64 -machine q35 -device pcie-root-port,id=3Dpcie-root-po= rt-0 -monitor stdio 9<> $TAP (qemu) netdev_add type=3Dtap,id=3Dhostnet0,vhost=3Don,fd=3D9 (qemu) device_add driver=3Dvirtio-net-pci,netdev=3Dhostnet0,id=3Dnet0,bus= =3Dpcie-root-port-0 (qemu) device_del net0 (qemu) netdev_del hostnet0 (qemu) netdev_add type=3Dtap,id=3Dhostnet1,vhost=3Don,fd=3D9 qemu-system-x86_64: .../util/oslib-posix.c:247: qemu_set_nonblock: Asserti= on `f !=3D -1' failed. Aborted (core dumped) To avoid that, add a function, qemu_try_set_nonblock(), that allows to repo= rt the problem without crashing. In the same way, we also update the function for vhostfd in net_init_tap_on= e() and for fd in net_init_socket() (both descriptors are provided by the user and = can be wrong). Signed-off-by: Laurent Vivier Reviewed-by: Philippe Mathieu-Daud=C3=A9 Signed-off-by: Jason Wang --- include/qemu/sockets.h | 1 + net/socket.c | 9 ++++++-- net/tap.c | 25 ++++++++++++++++++---- util/oslib-posix.c | 26 ++++++++++++++++------- util/oslib-win32.c | 57 ++++++++++++++++++++++++++++------------------= ---- 5 files changed, 79 insertions(+), 39 deletions(-) diff --git a/include/qemu/sockets.h b/include/qemu/sockets.h index 57cd049..7d1f813 100644 --- a/include/qemu/sockets.h +++ b/include/qemu/sockets.h @@ -18,6 +18,7 @@ int qemu_accept(int s, struct sockaddr *addr, socklen_t *= addrlen); int socket_set_cork(int fd, int v); int socket_set_nodelay(int fd); void qemu_set_block(int fd); +int qemu_try_set_nonblock(int fd); void qemu_set_nonblock(int fd); int socket_set_fast_reuse(int fd); =20 diff --git a/net/socket.c b/net/socket.c index c923540..2d21fdd 100644 --- a/net/socket.c +++ b/net/socket.c @@ -725,13 +725,18 @@ int net_init_socket(const Netdev *netdev, const char = *name, } =20 if (sock->has_fd) { - int fd; + int fd, ret; =20 fd =3D monitor_fd_param(cur_mon, sock->fd, errp); if (fd =3D=3D -1) { return -1; } - qemu_set_nonblock(fd); + ret =3D qemu_try_set_nonblock(fd); + if (ret < 0) { + error_setg_errno(errp, -ret, "%s: Can't use file descriptor %d= ", + name, fd); + return -1; + } if (!net_socket_fd_init(peer, "socket", name, fd, 1, sock->mcast, errp)) { return -1; diff --git a/net/tap.c b/net/tap.c index f9dcc2e..32e4813 100644 --- a/net/tap.c +++ b/net/tap.c @@ -690,6 +690,8 @@ static void net_init_tap_one(const NetdevTapOptions *ta= p, NetClientState *peer, } =20 if (vhostfdname) { + int ret; + vhostfd =3D monitor_fd_param(cur_mon, vhostfdname, &err); if (vhostfd =3D=3D -1) { if (tap->has_vhostforce && tap->vhostforce) { @@ -699,7 +701,12 @@ static void net_init_tap_one(const NetdevTapOptions *t= ap, NetClientState *peer, } return; } - qemu_set_nonblock(vhostfd); + ret =3D qemu_try_set_nonblock(vhostfd); + if (ret < 0) { + error_setg_errno(errp, -ret, "%s: Can't use file descripto= r %d", + name, fd); + return; + } } else { vhostfd =3D open("/dev/vhost-net", O_RDWR); if (vhostfd < 0) { @@ -767,6 +774,7 @@ int net_init_tap(const Netdev *netdev, const char *name, Error *err =3D NULL; const char *vhostfdname; char ifname[128]; + int ret =3D 0; =20 assert(netdev->type =3D=3D NET_CLIENT_DRIVER_TAP); tap =3D &netdev->u.tap; @@ -795,7 +803,12 @@ int net_init_tap(const Netdev *netdev, const char *nam= e, return -1; } =20 - qemu_set_nonblock(fd); + ret =3D qemu_try_set_nonblock(fd); + if (ret < 0) { + error_setg_errno(errp, -ret, "%s: Can't use file descriptor %d= ", + name, fd); + return -1; + } =20 vnet_hdr =3D tap_probe_vnet_hdr(fd); =20 @@ -810,7 +823,6 @@ int net_init_tap(const Netdev *netdev, const char *name, char **fds; char **vhost_fds; int nfds =3D 0, nvhosts =3D 0; - int ret =3D 0; =20 if (tap->has_ifname || tap->has_script || tap->has_downscript || tap->has_vnet_hdr || tap->has_helper || tap->has_queues || @@ -842,7 +854,12 @@ int net_init_tap(const Netdev *netdev, const char *nam= e, goto free_fail; } =20 - qemu_set_nonblock(fd); + ret =3D qemu_try_set_nonblock(fd); + if (ret < 0) { + error_setg_errno(errp, -ret, "%s: Can't use file descripto= r %d", + name, fd); + goto free_fail; + } =20 if (i =3D=3D 0) { vnet_hdr =3D tap_probe_vnet_hdr(fd); diff --git a/util/oslib-posix.c b/util/oslib-posix.c index e60aea8..36bf859 100644 --- a/util/oslib-posix.c +++ b/util/oslib-posix.c @@ -260,25 +260,35 @@ void qemu_set_block(int fd) assert(f !=3D -1); } =20 -void qemu_set_nonblock(int fd) +int qemu_try_set_nonblock(int fd) { int f; f =3D fcntl(fd, F_GETFL); - assert(f !=3D -1); - f =3D fcntl(fd, F_SETFL, f | O_NONBLOCK); -#ifdef __OpenBSD__ if (f =3D=3D -1) { + return -errno; + } + if (fcntl(fd, F_SETFL, f | O_NONBLOCK) =3D=3D -1) { +#ifdef __OpenBSD__ /* * Previous to OpenBSD 6.3, fcntl(F_SETFL) is not permitted on * memory devices and sets errno to ENODEV. * It's OK if we fail to set O_NONBLOCK on devices like /dev/null, * because they will never block anyway. */ - assert(errno =3D=3D ENODEV); - } -#else - assert(f !=3D -1); + if (errno =3D=3D ENODEV) { + return 0; + } #endif + return -errno; + } + return 0; +} + +void qemu_set_nonblock(int fd) +{ + int f; + f =3D qemu_try_set_nonblock(fd); + assert(f =3D=3D 0); } =20 int socket_set_fast_reuse(int fd) diff --git a/util/oslib-win32.c b/util/oslib-win32.c index 3b49d27..7eedbe5 100644 --- a/util/oslib-win32.c +++ b/util/oslib-win32.c @@ -132,31 +132,6 @@ struct tm *localtime_r(const time_t *timep, struct tm = *result) } #endif /* CONFIG_LOCALTIME_R */ =20 -void qemu_set_block(int fd) -{ - unsigned long opt =3D 0; - WSAEventSelect(fd, NULL, 0); - ioctlsocket(fd, FIONBIO, &opt); -} - -void qemu_set_nonblock(int fd) -{ - unsigned long opt =3D 1; - ioctlsocket(fd, FIONBIO, &opt); - qemu_fd_register(fd); -} - -int socket_set_fast_reuse(int fd) -{ - /* Enabling the reuse of an endpoint that was used by a socket still in - * TIME_WAIT state is usually performed by setting SO_REUSEADDR. On Wi= ndows - * fast reuse is the default and SO_REUSEADDR does strange things. So = we - * don't have to do anything here. More info can be found at: - * http://msdn.microsoft.com/en-us/library/windows/desktop/ms740621.as= px */ - return 0; -} - - static int socket_error(void) { switch (WSAGetLastError()) { @@ -233,6 +208,38 @@ static int socket_error(void) } } =20 +void qemu_set_block(int fd) +{ + unsigned long opt =3D 0; + WSAEventSelect(fd, NULL, 0); + ioctlsocket(fd, FIONBIO, &opt); +} + +int qemu_try_set_nonblock(int fd) +{ + unsigned long opt =3D 1; + if (ioctlsocket(fd, FIONBIO, &opt) !=3D NO_ERROR) { + return -socket_error(); + } + qemu_fd_register(fd); + return 0; +} + +void qemu_set_nonblock(int fd) +{ + (void)qemu_try_set_nonblock(fd); +} + +int socket_set_fast_reuse(int fd) +{ + /* Enabling the reuse of an endpoint that was used by a socket still in + * TIME_WAIT state is usually performed by setting SO_REUSEADDR. On Wi= ndows + * fast reuse is the default and SO_REUSEADDR does strange things. So = we + * don't have to do anything here. More info can be found at: + * http://msdn.microsoft.com/en-us/library/windows/desktop/ms740621.as= px */ + return 0; +} + int inet_aton(const char *cp, struct in_addr *ia) { uint32_t addr =3D inet_addr(cp); --=20 2.5.0