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: 9b8d05b8ac ("Add support for Amazon Elastic Network Adapter (ENA) NIC")
Fixes: 30217e2dff ("Rework counting of hardware statistics in ENA driver")
MFC after: 2 weeks
Sponsored by: Amazon, Inc.
This commit is contained in:
Osama Abboud 2023-01-18 13:19:07 +00:00 committed by Marcin Wojtas
parent 4ef3964b5f
commit b9e80b5280

View file

@ -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))