Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 31 Jul 2023 12:56:26 GMT
From:      Marcin Wojtas <mw@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org
Subject:   git: 383cf7052a29 - stable/12 - ena: Initialize statistics before the interface is available
Message-ID:  <202307311256.36VCuQIf093329@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch stable/12 has been updated by mw:

URL: https://cgit.FreeBSD.org/src/commit/?id=383cf7052a29ab3f4d1bdffb888696546e4f0fe4

commit 383cf7052a29ab3f4d1bdffb888696546e4f0fe4
Author:     Osama Abboud <osamaabb@amazon.com>
AuthorDate: 2023-01-18 13:19:07 +0000
Commit:     Marcin Wojtas <mw@FreeBSD.org>
CommitDate: 2023-07-31 12:49:47 +0000

    ena: Initialize statistics before the interface is available
    
    In [1], the FBSD community exposed a bug in the fbsd/ena driver.
    
    Bug description:
    ----------------
    Current function call order is as follows:
    
    1. ena_attach()
    1.1. ena_setup_ifnet()
    1.1.1. Registration of ena_get_counter()
    1.1.2. ether_ifattach(ifp, adapter->mac_addr);
    1.2. Statistics allocation and initialization.
    
    At point 1.1.2, when ether_ifattach() returns, the interface is available,
    and stats can be read before they are allocated, leading to kernel panic.
    
    Also fixed a potential memory leak by freeing the stats since they were
    not freed in case the following calls failed.
    
    Fix:
    ----
    This commit moves the statistics allocation and initialization to happen
    before ena_setup_ifnet()
    
    [1] https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=268934
    
    Fixes: 9b8d05b8ac78 ("Add support for Amazon Elastic Network Adapter (ENA) NIC")
    Fixes: 30217e2dff10 ("Rework counting of hardware statistics in ENA driver")
    MFC after: 2 weeks
    Sponsored by: Amazon, Inc.
    
    (cherry picked from commit b9e80b5280b75f2c641d680245df44b8ff26a7b0)
---
 sys/dev/ena/ena.c | 29 ++++++++++++++++++-----------
 1 file changed, 18 insertions(+), 11 deletions(-)

diff --git a/sys/dev/ena/ena.c b/sys/dev/ena/ena.c
index 15c8d39d8fb5..c8c12593fcd1 100644
--- a/sys/dev/ena/ena.c
+++ b/sys/dev/ena/ena.c
@@ -3484,6 +3484,15 @@ ena_reset_task(void *arg, int pending)
 	ENA_LOCK_UNLOCK();
 }
 
+static void
+ena_free_stats(struct ena_adapter *adapter)
+{
+	ena_free_counters((counter_u64_t *)&adapter->hw_stats,
+	    sizeof(struct ena_hw_stats));
+	ena_free_counters((counter_u64_t *)&adapter->dev_stats,
+	    sizeof(struct ena_stats_dev));
+
+}
 /**
  * ena_attach - Device Initialization Routine
  * @pdev: device information struct
@@ -3661,6 +3670,13 @@ ena_attach(device_t pdev)
 	/* initialize rings basic information */
 	ena_init_io_rings(adapter);
 
+	/* Initialize statistics */
+	ena_alloc_counters((counter_u64_t *)&adapter->dev_stats,
+	    sizeof(struct ena_stats_dev));
+	ena_alloc_counters((counter_u64_t *)&adapter->hw_stats,
+	    sizeof(struct ena_hw_stats));
+	ena_sysctl_add_nodes(adapter);
+
 	/* setup network interface */
 	rc = ena_setup_ifnet(pdev, adapter, &get_feat_ctx);
 	if (unlikely(rc != 0)) {
@@ -3682,13 +3698,6 @@ ena_attach(device_t pdev)
 	taskqueue_start_threads(&adapter->metrics_tq, 1, PI_NET, "%s metricsq",
 	    device_get_nameunit(adapter->pdev));
 
-	/* Initialize statistics */
-	ena_alloc_counters((counter_u64_t *)&adapter->dev_stats,
-	    sizeof(struct ena_stats_dev));
-	ena_alloc_counters((counter_u64_t *)&adapter->hw_stats,
-	    sizeof(struct ena_hw_stats));
-	ena_sysctl_add_nodes(adapter);
-
 #ifdef DEV_NETMAP
 	rc = ena_netmap_attach(adapter);
 	if (rc != 0) {
@@ -3711,6 +3720,7 @@ err_detach:
 	ether_ifdetach(adapter->ifp);
 #endif /* DEV_NETMAP */
 err_msix_free:
+	ena_free_stats(adapter);
 	ena_com_dev_reset(adapter->ena_dev, ENA_REGS_RESET_INIT_ERR);
 	ena_free_mgmnt_irq(adapter);
 	ena_disable_msix(adapter);
@@ -3783,10 +3793,7 @@ ena_detach(device_t pdev)
 	netmap_detach(adapter->ifp);
 #endif /* DEV_NETMAP */
 
-	ena_free_counters((counter_u64_t *)&adapter->hw_stats,
-	    sizeof(struct ena_hw_stats));
-	ena_free_counters((counter_u64_t *)&adapter->dev_stats,
-	    sizeof(struct ena_stats_dev));
+	ena_free_stats(adapter);
 
 	rc = ena_free_rx_dma_tag(adapter);
 	if (unlikely(rc != 0))



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202307311256.36VCuQIf093329>