git: 8a6ba4eefb66 - stable/13 - ena: Initialize statistics before the interface is available

From: Marcin Wojtas <mw_at_FreeBSD.org>
Date: Mon, 31 Jul 2023 12:26:16 UTC
The branch stable/13 has been updated by mw:

URL: https://cgit.FreeBSD.org/src/commit/?id=8a6ba4eefb66159c092330576e9e50748b4db737

commit 8a6ba4eefb66159c092330576e9e50748b4db737
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:23:34 +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 4dcbb709c586..a974a20638b9 100644
--- a/sys/dev/ena/ena.c
+++ b/sys/dev/ena/ena.c
@@ -3480,6 +3480,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
@@ -3657,6 +3666,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)) {
@@ -3678,13 +3694,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) {
@@ -3707,6 +3716,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);
@@ -3779,10 +3789,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))