Re: [PATCH 2/3] net: mana: Skip WQ object destruction for uninitialized RXQ

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] Critical Use-After-Free and Double-Free in `mana_create_rxq()` error path.
- [High] Incomplete Fix: The identical bug fixed in `mana_destroy_rxq()` exists in `mana_destroy_txq()`.
--

commit c317e37e7bd422dc85e0c08611eda7f514d25eef
Author: Dipayaan Roy <dipayanroy@linux.microsoft.com>

net: mana: Skip WQ object destruction for uninitialized RXQ

This commit adds a check for INVALID_MANA_HANDLE in mana_destroy_rxq()
to prevent sending a bogus destroy command to the hardware when
mana_create_rxq() fails before the WQ object is created.

> diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c
> index dfb4ba9f7664..f2a6ea162dc3 100644
> --- a/drivers/net/ethernet/microsoft/mana/mana_en.c
> +++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
> @@ -2524,7 +2524,8 @@ static void mana_destroy_rxq(struct mana_port_context *apc,
>  	if (xdp_rxq_info_is_reg(&rxq->xdp_rxq))
>  		xdp_rxq_info_unreg(&rxq->xdp_rxq);
>  
> -	mana_destroy_wq_obj(apc, GDMA_RQ, rxq->rxobj);
> +	if (rxq->rxobj != INVALID_MANA_HANDLE)
> +		mana_destroy_wq_obj(apc, GDMA_RQ, rxq->rxobj);

Does this incomplete fix leave a regression in the TX queue destruction path?

In mana_create_txq(), apc->tx_qp is allocated via kzalloc_objs(), defaulting
tx_object to 0. It is set to INVALID_MANA_HANDLE only at the start of each
queue's loop iteration.

If mana_create_txq() fails early, it calls mana_destroy_txq(apc), which
unconditionally iterates over all apc->num_queues and calls:

    mana_destroy_wq_obj(apc, GDMA_SQ, apc->tx_qp[i].tx_object);

For the failing queue, it sends a destroy command for INVALID_MANA_HANDLE,
and for unreached queues, it sends 0. Should mana_destroy_txq() conditionally
check if tx_object is initialized before destroying it, similar to this fix?

While looking at this error path, I noticed a pre-existing regression that
is noted as fixed in a later commit (fbbff2b6d2993ad net: mana: remove
double CQ cleanup in mana_create_rxq error path).

In mana_create_rxq(), a local pointer cq is assigned to &rxq->rx_cq. If an
error occurs, it calls mana_destroy_rxq(), which frees rxq. Immediately
after, the error path checks if (cq) and calls mana_deinit_cq(apc, cq).

Since cq points into the freed rxq memory, does this result in a
use-after-free and double-free when mana_deinit_cq() reads cq->gdma_cq?

>  
>  	mana_deinit_cq(apc, &rxq->rx_cq);

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