From owner-dev-commits-src-branches@freebsd.org Tue Sep 28 17:30:11 2021 Return-Path: Delivered-To: dev-commits-src-branches@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 38B266A8E04; Tue, 28 Sep 2021 17:30:11 +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 4HJmjb0qYwz4Tf8; Tue, 28 Sep 2021 17:30:11 +0000 (UTC) (envelope-from git@FreeBSD.org) 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 E8E71733; Tue, 28 Sep 2021 17:30:10 +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 18SHUAL7079484; Tue, 28 Sep 2021 17:30:10 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 18SHUAAh079480; Tue, 28 Sep 2021 17:30:10 GMT (envelope-from git) Date: Tue, 28 Sep 2021 17:30:10 GMT Message-Id: <202109281730.18SHUAAh079480@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org From: Kevin Bowling Subject: git: 5b4cfd3f1315 - stable/12 - iflib: Free resources in a consistent order during detach MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: kbowling X-Git-Repository: src X-Git-Refname: refs/heads/stable/12 X-Git-Reftype: branch X-Git-Commit: 5b4cfd3f1315856029256a9847d78d3b1b717fb0 Auto-Submitted: auto-generated X-BeenThere: dev-commits-src-branches@freebsd.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Commits to the stable branches of the FreeBSD src repository List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 28 Sep 2021 17:30:11 -0000 The branch stable/12 has been updated by kbowling (ports committer): URL: https://cgit.FreeBSD.org/src/commit/?id=5b4cfd3f1315856029256a9847d78d3b1b717fb0 commit 5b4cfd3f1315856029256a9847d78d3b1b717fb0 Author: Sai Rajesh Tallamraju AuthorDate: 2021-02-01 16:13:00 +0000 Commit: Kevin Bowling CommitDate: 2021-09-28 17:21:52 +0000 iflib: Free resources in a consistent order during detach Memory and PCI resources are freed with no particular order. This could cause use-after-frees when detaching following a failed attach. For instance, iflib_tx_structures_free() frees ctx->ifc_txqs[] but iflib_tqg_detach() attempts to access this array. Similarly, adapter queues gets freed by IFDI_QUEUES_FREE() but IFDI_DETACH() attempts to access adapter queues to free PCI resources. MFC after: 2 weeks Sponsored by: NetApp, Inc. Differential Revision: https://reviews.freebsd.org/D27634 (cherry picked from commit 38bfc6dee33bedb290e1ea2540f97a86fe3caee0) --- sys/dev/e1000/if_em.c | 19 ++++++------------- sys/dev/ixl/if_ixl.c | 2 +- sys/net/iflib.c | 22 +++++++++++++--------- 3 files changed, 20 insertions(+), 23 deletions(-) diff --git a/sys/dev/e1000/if_em.c b/sys/dev/e1000/if_em.c index a571450a463e..7babe51c8c02 100644 --- a/sys/dev/e1000/if_em.c +++ b/sys/dev/e1000/if_em.c @@ -1127,10 +1127,11 @@ em_if_attach_post(if_ctx_t ctx) struct adapter *adapter = iflib_get_softc(ctx); struct e1000_hw *hw = &adapter->hw; int error = 0; - + /* Setup OS specific network interface */ error = em_setup_interface(ctx); if (error != 0) { + device_printf(adapter->dev, "Interface setup failed: %d\n", error); goto err_late; } @@ -1148,14 +1149,10 @@ em_if_attach_post(if_ctx_t ctx) INIT_DEBUGOUT("em_if_attach_post: end"); - return (error); + return (0); err_late: - em_release_hw_control(adapter); - em_free_pci_resources(ctx); - em_if_queues_free(ctx); - free(adapter->mta, M_DEVBUF); - + /* upon attach_post() error, iflib calls _if_detach() to free resources. */ return (error); } @@ -1180,6 +1177,8 @@ em_if_detach(if_ctx_t ctx) em_release_manageability(adapter); em_release_hw_control(adapter); em_free_pci_resources(ctx); + free(adapter->mta, M_DEVBUF); + adapter->mta = NULL; return (0); } @@ -3012,12 +3011,6 @@ em_if_queues_free(if_ctx_t ctx) free(adapter->rx_queues, M_DEVBUF); adapter->rx_queues = NULL; } - - em_release_hw_control(adapter); - - if (adapter->mta != NULL) { - free(adapter->mta, M_DEVBUF); - } } /********************************************************************* diff --git a/sys/dev/ixl/if_ixl.c b/sys/dev/ixl/if_ixl.c index 8741eb06ac18..45ac55ec2da8 100644 --- a/sys/dev/ixl/if_ixl.c +++ b/sys/dev/ixl/if_ixl.c @@ -1275,7 +1275,7 @@ ixl_if_queues_free(if_ctx_t ctx) struct ixl_pf *pf = iflib_get_softc(ctx); struct ixl_vsi *vsi = &pf->vsi; - if (!vsi->enable_head_writeback) { + if (vsi->tx_queues != NULL && !vsi->enable_head_writeback) { struct ixl_tx_queue *que; int i = 0; diff --git a/sys/net/iflib.c b/sys/net/iflib.c index f8d4c120b3ec..2d53b1ef8229 100644 --- a/sys/net/iflib.c +++ b/sys/net/iflib.c @@ -5157,7 +5157,7 @@ iflib_device_register(device_t dev, void *sc, if_shared_ctx_t sctx, if_ctx_t *ct device_printf(dev, "Cannot use iflib with only 1 MSI-X interrupt!\n"); err = ENODEV; - goto fail_intr_free; + goto fail_queues; } ether_ifattach(ctx->ifc_ifp, ctx->ifc_mac); @@ -5192,13 +5192,14 @@ iflib_device_register(device_t dev, void *sc, if_shared_ctx_t sctx, if_ctx_t *ct fail_detach: ether_ifdetach(ctx->ifc_ifp); -fail_intr_free: - iflib_free_intr_mem(ctx); fail_queues: + iflib_tqg_detach(ctx); iflib_tx_structures_free(ctx); iflib_rx_structures_free(ctx); - iflib_tqg_detach(ctx); IFDI_DETACH(ctx); + IFDI_QUEUES_FREE(ctx); +fail_intr_free: + iflib_free_intr_mem(ctx); fail_unlock: CTX_UNLOCK(ctx); iflib_deregister(ctx); @@ -5397,11 +5398,12 @@ iflib_pseudo_register(device_t dev, if_shared_ctx_t sctx, if_ctx_t *ctxp, fail_detach: ether_ifdetach(ctx->ifc_ifp); fail_queues: + iflib_tqg_detach(ctx); iflib_tx_structures_free(ctx); iflib_rx_structures_free(ctx); - iflib_tqg_detach(ctx); fail_iflib_detach: IFDI_DETACH(ctx); + IFDI_QUEUES_FREE(ctx); fail_unlock: CTX_UNLOCK(ctx); iflib_deregister(ctx); @@ -5431,6 +5433,8 @@ iflib_pseudo_deregister(if_ctx_t ctx) iflib_tqg_detach(ctx); iflib_tx_structures_free(ctx); iflib_rx_structures_free(ctx); + IFDI_DETACH(ctx); + IFDI_QUEUES_FREE(ctx); iflib_deregister(ctx); @@ -5490,8 +5494,12 @@ iflib_device_deregister(if_ctx_t ctx) led_destroy(ctx->ifc_led_dev); iflib_tqg_detach(ctx); + iflib_tx_structures_free(ctx); + iflib_rx_structures_free(ctx); + CTX_LOCK(ctx); IFDI_DETACH(ctx); + IFDI_QUEUES_FREE(ctx); CTX_UNLOCK(ctx); /* ether_ifdetach calls if_qflush - lock must be destroy afterwards*/ @@ -5499,9 +5507,6 @@ iflib_device_deregister(if_ctx_t ctx) bus_generic_detach(dev); - iflib_tx_structures_free(ctx); - iflib_rx_structures_free(ctx); - iflib_deregister(ctx); device_set_softc(ctx->ifc_dev, NULL); @@ -6084,7 +6089,6 @@ iflib_tx_structures_free(if_ctx_t ctx) } free(ctx->ifc_txqs, M_IFLIB); ctx->ifc_txqs = NULL; - IFDI_QUEUES_FREE(ctx); } /*********************************************************************