Re: [PATCH net v3 3/6] net: mana: free HWC comp_buf after destroying the EQ

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Destroying the TX and RX queues before the CQ and EQ in mana_hwc_destroy_channel() causes a use-after-free if a late completion interrupt is processed.
--

--- Patch [3]: [PATCH net v3 3/6] net: mana: free HWC comp_buf after destroying the EQ ---
commit 7a0285e343813287658afb74760a65202a46b1ba
Author: Long Li <longli@microsoft.com>

net: mana: free HWC comp_buf after destroying the EQ

This commit fixes a use-after-free by destroying the CQ and EQ before
freeing the hwc_cq->comp_buf buffer. The EQ teardown deregisters the IRQ
and fences in-flight handlers, preventing late handlers from touching
freed memory.

> diff --git a/drivers/net/ethernet/microsoft/mana/hw_channel.c b/drivers/net/ethernet/microsoft/mana/hw_channel.c
> index cbb56c7647876..d701c427fe47b 100644
> --- a/drivers/net/ethernet/microsoft/mana/hw_channel.c
> +++ b/drivers/net/ethernet/microsoft/mana/hw_channel.c
[ ... ]
> @@ -384,14 +384,20 @@ static void mana_hwc_comp_event(void *ctx, struct gdma_queue *q_self)
>  
>  static void mana_hwc_destroy_cq(struct gdma_context *gc, struct hwc_cq *hwc_cq)
>  {
> -	kfree(hwc_cq->comp_buf);
> -
>  	if (hwc_cq->gdma_cq)
>  		mana_gd_destroy_queue(gc, hwc_cq->gdma_cq);
>  
> +	/* comp_buf is reached only by mana_hwc_comp_event(), which the
> +	 * EQ handler invokes via cq_table[id].  The CQ destroy above
> +	 * already cleared that slot and ran synchronize_rcu(), so no
> +	 * handler can reach comp_buf once it returns.  Destroying the EQ
> +	 * here additionally tears down the IRQ (defense in depth) before
> +	 * comp_buf and hwc_cq are freed below.
> +	 */
>  	if (hwc_cq->gdma_eq)
>  		mana_gd_destroy_queue(gc, hwc_cq->gdma_eq);
>  
> +	kfree(hwc_cq->comp_buf);
>  	kfree(hwc_cq);
>  }

[Severity: High]
This is a pre-existing issue, but does the same use-after-free race still
exist for the TX and RX queues during channel teardown?

While this commit fixes the free order for comp_buf,
mana_hwc_destroy_channel() still frees the WQs before the CQ and EQ:

mana_hwc_destroy_channel()
{
        if (hwc->txq)
                mana_hwc_destroy_wq(hwc, hwc->txq);

        if (hwc->rxq)
                mana_hwc_destroy_wq(hwc, hwc->rxq);

        if (hwc->cq)
                mana_hwc_destroy_cq(hwc->gdma_dev->gdma_context, hwc->cq);
}

If a late EQ interrupt arrives or is already running in the window between
WQ destruction and CQ destruction, won't mana_gd_process_eqe() still find
the CQ in cq_table and invoke mana_hwc_comp_event()?

This would then call into mana_hwc_rx_event_handler() or
mana_hwc_tx_event_handler() and dereference the already-freed hwc->rxq
or hwc->txq memory:

mana_hwc_rx_event_handler()
{
        struct hwc_wq *hwc_rxq = hwc->rxq;
        ...
        if (WARN_ON_ONCE(hwc_rxq->gdma_wq->id != gdma_rxq_id))
                return;
        ...
}

Could the WQ destruction in mana_hwc_destroy_channel() be moved after the
CQ and EQ are destroyed to ensure all in-flight handlers are fenced before
the WQ memory is freed?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260803234355.636038-1-longli@microsoft.com?part=3