Re: [PATCH net-next v3 10/13] net: mana: release EQs left idle by a channel-count reduction

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Use-After-Free of CQ in EQ event processing due to lack of synchronization during CQ destruction
--

commit 3d0d6fcf405f01d902d109c4441be576c316986f
Author: Long Li <longli@microsoft.com>
net: mana: release EQs left idle by a channel-count reduction

The shared EQ pool only grows, so it sits at the high-water mark of every
channel count the port has ever used. This commit releases the EQs above the
live queue count once a retiring set has been torn down. It also ensures the
dentry is properly recorded for debugfs.

> diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c
> index 69e34fab8fe8e..2295769da0306 100644
> --- a/drivers/net/ethernet/microsoft/mana/mana_en.c
> +++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
[ ... ]
> @@ -4745,12 +4785,19 @@ void mana_free_qset(struct mana_port_context *scratch, struct mana_qset *qset)
>  
>  	memset(qset, 0, sizeof(*qset));
>  
> +	/* This set is gone, so any EQ above the live queue count is now
> +	 * unreferenced. Release those vectors instead of holding them at the
> +	 * high-water mark. Safe here and only here: the retiring set's CQs
> +	 * have just been destroyed.
> +	 */

[Severity: High]
This is a pre-existing issue, but since this teardown path is being modified,
can the destruction of the retiring set's CQs race with the EQ interrupt
handler?

An admin reconfiguring the network interface (e.g., changing MTU, ring sizes,
or channels) will tear down the old queue set. The hardware queues are fenced,
generating a final CQE in the CQ.

If the EQ interrupt fires, CPU A could process the EQE in
mana_gd_process_eqe(). It reads the CQ pointer but might be delayed (e.g., by
an NMI) before invoking the callback:

drivers/net/ethernet/microsoft/mana/gdma_main.c:mana_gd_process_eqe() {
    ...
    cq = gc->cq_table[cq_id];
    if (WARN_ON_ONCE(!cq || cq->type != GDMA_CQ || cq->id != cq_id))
        break;

    if (cq->cq.callback)
        cq->cq.callback(cq->cq.context, cq);
    ...
}

Concurrently, NAPI on CPU C could process the fence CQE and signal the
completion event. CPU B, waiting in mana_fence_rqs(), wakes up and tears down
the old CQ via mana_destroy_rxq() -> mana_gd_destroy_queue().

It clears gc->cq_table[cq_id] = NULL in mana_gd_destroy_cq() and frees the CQ
memory immediately in mana_gd_destroy_queue():

drivers/net/ethernet/microsoft/mana/gdma_main.c:mana_gd_destroy_queue() {
    ...
    mana_gd_destroy_cq(gc, queue);
    ...
    kfree(queue);
}

When CPU A resumes and executes cq->cq.callback(cq->cq.context, cq), this
dereferences the freed cq and mana_rxq structures to call napi_schedule(),
causing a UAF write to napi.state.

Is there any synchronization, such as RCU or IRQ synchronization, to prevent
this use-after-free during CQ destruction?

> +	mana_shrink_eqs(apc, apc->num_queues);
> +
>  	/* Queues built through a scratch context carry no debugfs nodes,
>  	 * because both sets are alive during the swap and would collide on
>  	 * the same names. The retiring set's nodes are gone now, so the
>  	 * published queues can finally take those names.
>  	 */
> -	mana_qset_debugfs_publish(netdev_priv(scratch->ndev));
> +	mana_qset_debugfs_publish(apc);
>  }
>  
>  /* --- end of pre-allocate + swap reconfiguration path ---------------------- */

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