The branch main has been updated by mw:
URL:
https://cgit.FreeBSD.org/src/commit/?id=b9e80b5280b75f2c641d680245df44b8ff26a7b0
commit b9e80b5280b75f2c641d680245df44b8ff26a7b0
Author: Osama Abboud <osama...@amazon.com>
AuthorDate: 2023-01-18 13:19:07 +0000
Commit: Marcin Wojtas <m...@freebsd.org>
CommitDate: 2023-07-04 13:51:16 +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.
---
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 a4762ce9ebb1..a62b99d5ee1b 100644
--- a/sys/dev/ena/ena.c
+++ b/sys/dev/ena/ena.c
@@ -3479,6 +3479,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
@@ -3656,6 +3665,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)) {
@@ -3677,13 +3693,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) {
@@ -3706,6 +3715,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);
@@ -3778,10 +3788,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))
