Thread (13 messages) 13 messages, 2 authors, 5d ago

Re: [PATCH net-next 5/7] net: mana: fix HWC teardown safety with setup_active flag and destroy ordering

From: sashiko-bot@kernel.org
Date: 2026-07-16 03:30:21

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing teardown synchronization for dynamically allocated service tasks leads to Use-After-Free during device removal.
--
--- Patch [5]: [PATCH net-next 5/7] net: mana: fix HWC teardown safety with setup_active flag and destroy ordering ---
commit d335b5db9ecdafc6371e77ac2bb32f6aa7467088
Author: Long Li [off-list ref]

net: mana: fix HWC teardown safety with setup_active flag and destroy ordering

Two teardown hazards let the hardware touch memory the driver freed.
First, if establish_channel fails, the caller had no reliable way to know
teardown was required and could free buffers while mappings were still live.
Second, destroy_channel freed TXQ/RXQ buffers while the HWC EQ was still on
the interrupt dispatch list.

Fixes: ca9c54d2d6a5 ("net: mana: Add a driver for Microsoft Azure Network Adapter (MANA)")
quoted hunk ↗ jump to hunk
diff --git a/drivers/net/ethernet/microsoft/mana/hw_channel.c b/drivers/net/ethernet/microsoft/mana/hw_channel.c
index 68236727aee8b..b26c2122ebf52 100644
--- a/drivers/net/ethernet/microsoft/mana/hw_channel.c
+++ b/drivers/net/ethernet/microsoft/mana/hw_channel.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -878,11 +902,62 @@ void mana_hwc_destroy_channel(struct gdma_context *gc)
 	if (!hwc)
 		return;
 
-	/* gc->max_num_cqs is set in mana_hwc_init_event_handler(). If it's
-	 * non-zero, the HWC worked and we should tear down the HWC here.
+	/* Tear down the HWC if setup_hwc previously activated MST entries.
+	 * This is the definitive flag — unlike max_num_cqs which depends
+	 * on the init EQE arriving.
 	 */
-	if (gc->max_num_cqs > 0)
-		mana_smc_teardown_hwc(&gc->shm_channel, false);
+	if (hwc->setup_active) {
+		int td_err = mana_smc_teardown_hwc(&gc->shm_channel, false);
[Severity: High]
This is a pre-existing issue, but is there a missing synchronization step for
dynamically allocated service tasks during device teardown?

When a hardware event (like GDMA_EQE_HWC_RESET_REQUEST) occurs, the EQ
interrupt handler calls mana_schedule_serv_work(), which allocates a
mana_serv_work structure and schedules it on the system workqueue:

drivers/net/ethernet/microsoft/mana/gdma_main.c:mana_schedule_serv_work() {
    ...
    mns_wk = kzalloc(sizeof(*mns_wk), GFP_ATOMIC);
    ...
    INIT_WORK(&mns_wk->serv_work, mana_serv_func);
    schedule_work(&mns_wk->serv_work);
}

If a device unbind or module removal occurs concurrently, mana_gd_remove()
is called, which frees the context without flushing or canceling these
dynamically allocated tasks:

drivers/net/ethernet/microsoft/mana/gdma_main.c:mana_gd_remove() {
    ...
    vfree(gc);
}

When the scheduled mana_serv_func() eventually executes, it accesses the
freed gdma_context (gc) and hw_channel_context (hwc):

drivers/net/ethernet/microsoft/mana/gdma_main.c:mana_serv_reset() {
    ...
    struct gdma_context *gc = pci_get_drvdata(pdev);
    struct hw_channel_context *hwc;
    ...
    hwc = gc->hwc.driver_data;
}

Could this race condition result in a Use-After-Free during teardown?
+
+		if (td_err) {
+			dev_err(gc->dev, "HWC teardown failed: %d, issuing FLR\n",
+				td_err);
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260715032942.3945317-1-longli@microsoft.com?part=5
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help