From nobody Tue Jun 07 10:30:01 2022 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 032FB5D2117; Tue, 7 Jun 2022 10:30:02 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4LHRST6Vbzz4jcr; Tue, 7 Jun 2022 10:30:01 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1654597801; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=x1yX3pJJw05l8AoEgVVrxwS3gJ3nI16n/stT0mfeuiU=; b=MOPMWeIrbBryZIp38AoSeU7EaKu1mjedrOzH16Zm+kAZk2CESCy+kjpZVZgkRJ6k4SGMmq HHXH6m36chcerL4ewRdx3LZpyMoaCNJ220lNtrQsvd5Mw5Gv44WfeDy0PSzzIN6x0EXrb5 c5s/R8RCBN/b03NbqjCnr+QSCwndbFV28obR3VpDmD5BUQ0nBY+lFJu+V8zXvhIITNNyQs 0xDPtNOqOSf9nnCqJFo7s62QON4J6Tnn0epJuQfdJ+hbCNaV6Q9fBEZWeXQpxpnDWD/bwd 30CJ2UbgwifD3ue9Ie4bK4S+nSNfySEXCba1b1Jc37MWT+uBkC3X34QFHncCXQ== Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id BB61D18C70; Tue, 7 Jun 2022 10:30:01 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.16.1/8.16.1) with ESMTP id 257AU1ME096875; Tue, 7 Jun 2022 10:30:01 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 257AU1C4096859; Tue, 7 Jun 2022 10:30:01 GMT (envelope-from git) Date: Tue, 7 Jun 2022 10:30:01 GMT Message-Id: <202206071030.257AU1C4096859@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: =?utf-8?Q?Roger Pau Monn=C3=A9?= Subject: git: f3d54ded282c - main - xenbus: improve device tracking List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: dev-commits-src-all@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: royger X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: f3d54ded282ce992d168052d2c428e9df2a41e28 Auto-Submitted: auto-generated ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1654597801; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=x1yX3pJJw05l8AoEgVVrxwS3gJ3nI16n/stT0mfeuiU=; b=ufRNkSFiF3yTjnaW+HiV7EkFw4nbVfoMflD3jXJLevSaFXM27o34YMavPqR/X7uSrjhCQl wa0CHeDMwdU21kzpuevlC9TY31saONnaW4sXmniU/Ja+TQjVGJ+pkxpSo9VgKP5TmLVuDZ 9MUm0ECOrYaZZ9JlnnEXNmqfHjC7jpWGP+kFt34r7i+4Mjy3vQisj+NErfSFG+RHviIVIe 0/bj97kimUl0sPfNxYm7cPKDLl04rpz2d/PeLjFVmy+djmMnp3Mcc3kHzpovFKGFl16FRc TNHE4ODvG9stkNczDNeHjaV9G3rFkSEBN9Q3YekCiKEIfAHwr2RmR1JH8GitTg== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1654597801; a=rsa-sha256; cv=none; b=b6NoF+q1LR1KR9P51s5korEPmnC2pL+dngThr+n2Sjx5lBtqCbnw5jS4TPUI8EBEW5FGUr q3FuY+8Kejv6mQg61u+8vP7fMIu593UTDpES+Ii2I8WjLOQGPeyI4PV/EEOhu7wU1UPXGU ekCER9fEjsZIQBgRbXHYM92QDVxyiGXeR1Gy+ldo4PtvhT2yEVDwuq90/B0/KuUMD7+xYp BLkOEotyuyRt0mOVZJYsNK7fhlrargEc470HXAQVLsHEtByCn8zpyTouPAjnUmewZGV9sG 8H8L2Zf9UGwq29JCHtyyQnr70k7CrDziBQREPQjOMJVo4Lbla8e/St3eBxCmAA== ARC-Authentication-Results: i=1; mx1.freebsd.org; none X-ThisMailContainsUnwantedMimeParts: N The branch main has been updated by royger: URL: https://cgit.FreeBSD.org/src/commit/?id=f3d54ded282ce992d168052d2c428e9df2a41e28 commit f3d54ded282ce992d168052d2c428e9df2a41e28 Author: Roger Pau Monné AuthorDate: 2022-03-21 11:47:20 +0000 Commit: Roger Pau Monné CommitDate: 2022-06-07 10:29:53 +0000 xenbus: improve device tracking xenbus needs to keep track of the devices exposed on xenstore, so that it can trigger frontend and backend device creation. Removal of backend devices is currently detected by checking the existence of the device (backend) xenstore directory, but that's prone to races as the device driver would usually add entries to such directory itself, so under certain circumstances it's possible for a driver to add node to the directory after the toolstack has removed it. This leads to devices not removed, which can eventually exhaust the memory of FreeBSD. Fix this by checking for the existence of the 'state' node instead of the directory, as such node will always be present when a device is active, and will be removed by the toolstack when the device is shut down. In order to avoid any races with the updating of the 'state' node by FreeBSD and the toolstack removing it use a transaction in xenbusb_write_ivar() for that purpose. Reported by: Ze Dupsys Sponsored by: Citrix Systems R&D --- sys/xen/xenbus/xenbusb.c | 67 +++++++++++++++++++++++++++++++----------------- 1 file changed, 43 insertions(+), 24 deletions(-) diff --git a/sys/xen/xenbus/xenbusb.c b/sys/xen/xenbus/xenbusb.c index e026f8203ea1..bd6ff552c9a6 100644 --- a/sys/xen/xenbus/xenbusb.c +++ b/sys/xen/xenbus/xenbusb.c @@ -254,7 +254,7 @@ xenbusb_delete_child(device_t dev, device_t child) static void xenbusb_verify_device(device_t dev, device_t child) { - if (xs_exists(XST_NIL, xenbus_get_node(child), "") == 0) { + if (xs_exists(XST_NIL, xenbus_get_node(child), "state") == 0) { /* * Device tree has been removed from Xenbus. * Tear down the device. @@ -907,6 +907,7 @@ xenbusb_write_ivar(device_t dev, device_t child, int index, uintptr_t value) case XENBUS_IVAR_STATE: { int error; + struct xs_transaction xst; newstate = (enum xenbus_state)value; sx_xlock(&ivars->xd_lock); @@ -915,31 +916,39 @@ xenbusb_write_ivar(device_t dev, device_t child, int index, uintptr_t value) goto out; } - error = xs_scanf(XST_NIL, ivars->xd_node, "state", - NULL, "%d", &currstate); - if (error) - goto out; - do { - error = xs_printf(XST_NIL, ivars->xd_node, "state", - "%d", newstate); - } while (error == EAGAIN); - if (error) { - /* - * Avoid looping through xenbus_dev_fatal() - * which calls xenbus_write_ivar to set the - * state to closing. - */ - if (newstate != XenbusStateClosing) - xenbus_dev_fatal(dev, error, - "writing new state"); - goto out; - } + error = xs_transaction_start(&xst); + if (error != 0) + goto out; + + do { + error = xs_scanf(xst, ivars->xd_node, "state", + NULL, "%d", &currstate); + } while (error == EAGAIN); + if (error) + goto out; + + do { + error = xs_printf(xst, ivars->xd_node, "state", + "%d", newstate); + } while (error == EAGAIN); + if (error) { + /* + * Avoid looping through xenbus_dev_fatal() + * which calls xenbus_write_ivar to set the + * state to closing. + */ + if (newstate != XenbusStateClosing) + xenbus_dev_fatal(dev, error, + "writing new state"); + goto out; + } + } while (xs_transaction_end(xst, 0)); ivars->xd_state = newstate; - if ((ivars->xd_flags & XDF_CONNECTING) != 0 - && (newstate == XenbusStateClosed - || newstate == XenbusStateConnected)) { + if ((ivars->xd_flags & XDF_CONNECTING) != 0 && + (newstate == XenbusStateClosed || + newstate == XenbusStateConnected)) { struct xenbusb_softc *xbs; ivars->xd_flags &= ~XDF_CONNECTING; @@ -949,8 +958,18 @@ xenbusb_write_ivar(device_t dev, device_t child, int index, uintptr_t value) wakeup(&ivars->xd_state); out: + if (error != 0) + xs_transaction_end(xst, 1); sx_xunlock(&ivars->xd_lock); - return (error); + /* + * Shallow ENOENT errors, as returning an error here will + * trigger a panic. ENOENT is fine to ignore, because it means + * the toolstack has removed the state node as part of + * destroying the device, and so we have to shut down the + * device without recreating it or else the node would be + * leaked. + */ + return (error == ENOENT ? 0 : error); } case XENBUS_IVAR_NODE: