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

[sashiko-bot@kernel.org]

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?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261014134304.4177657-1-sahilchandna@linux.microsoft.com?part=1