Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] net: mana: use-after-free of CQ during teardown due to RCU-001 removal ordering violation --
commit 4c00972dfa8283dfc036bc3bbb77031cb6550854 Author: Sahil Chandna <[email protected]> net: mana: Add support to process throttled EQEs When an event queue nears full, the hardware coalesces the per-CQ GDMA_EQE_COMPLETION notifications into a single throttle EQE (type 4) that carries no cq_id. The driver does not handle this event type, so it falls through to the default case and is dropped. Any CQ whose completion notification was replaced by the throttle EQE can remain unserviced, causing its queue to stall. Link: https://lore.kernel.org/all/[email protected]/ > diff --git a/drivers/net/ethernet/microsoft/mana/gdma_main.c > b/drivers/net/ethernet/microsoft/mana/gdma_main.c > index ae0ed700b3b93..95ba6ba161ae2 100644 > --- a/drivers/net/ethernet/microsoft/mana/gdma_main.c > +++ b/drivers/net/ethernet/microsoft/mana/gdma_main.c [ ... ] > @@ -1063,17 +1089,45 @@ static void mana_gd_create_cq(const struct > gdma_queue_spec *spec, > static void mana_gd_destroy_cq(struct gdma_context *gc, > struct gdma_queue *queue) > { > + struct gdma_queue *eq = queue->cq.parent; > + bool attached = false; > u32 id = queue->id; > + u8 i; > + > + if (eq) { > + for (i = 0; i < GDMA_EQ_MAX_CHILD_CQ; i++) { > + if (rcu_access_pointer(eq->eq.child_cq[i]) == queue) { > + RCU_INIT_POINTER(eq->eq.child_cq[i], NULL); > + attached = true; > + } > + } > + > + if (attached) > + synchronize_net(); > + } > > if (id >= gc->max_num_cqs) > return; [Severity: High] Does this code order the CQ removal and grace period correctly to prevent a use-after-free? Looking at the rest of mana_gd_destroy_cq(), gc->cq_table[id] is cleared after the synchronize_net() grace period ends: drivers/net/ethernet/microsoft/mana/gdma_main.c:mana_gd_destroy_cq() { if (eq) { ... if (attached) synchronize_net(); } if (id >= gc->max_num_cqs) return; if (!gc->cq_table[id]) return; gc->cq_table[id] = NULL; } Because gc->cq_table[id] remains published during and immediately after the grace period, can a concurrent or delayed interrupt processing GDMA_EQE_COMPLETION in mana_gd_process_eqe() still find the CQ? drivers/net/ethernet/microsoft/mana/gdma_main.c:mana_gd_process_eqe() { case GDMA_EQE_COMPLETION: ... cq = gc->cq_table[cq_id]; ... if (cq->cq.callback) cq->cq.callback(cq->cq.context, cq); } If a new reader accesses the CQ from gc->cq_table after synchronize_net() returns, but before the caller mana_gd_destroy_queue() frees the memory via kfree(), could it lead to a use-after-free crash? Should the clearing of gc->cq_table[id] happen before calling synchronize_net() so all reader-visible references are removed before waiting for the grace period? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
