Re: [PATCH net v2] net: mana: Add support to process throttled EQEs

[Sahil Chandna <sahilchandna@linux.microsoft.com>]

On 15-10-2026 19:13, sashiko-bot@kernel.org 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 <sahilchandna@linux.microsoft.com>
> 
> 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/20261008083315.306076-1-sahilchandna@linux.microsoft.com/
> 
>> 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.