On 15-10-2026 19:13, [email protected] wrote:
> 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?
>
The cq_table dereferencing race is real between mana_gd_destroy_cq and
in GDMA_EQE_COMPLETION. I will address this re-ordering and if there are
any other comments on this patch in v3.
