Re: [PATCH net v2 1/2] net: mana: initialize gdma queue id to INVALID_QUEUE_ID

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The mana_gd_create_mana_eq function leaks a firmware DMA region object on its error path if the EQ hardware creation fails.
--

commit 4c25a3c164c64bd1c7eed40e10b6c6526c99eb4c
Author: Aditya Garg <gargaditya@linux.microsoft.com>

net: mana: initialize gdma queue id to INVALID_QUEUE_ID

mana_gd_create_mana_wq_cq() leaves queue->id as 0 (from kzalloc_obj())
until mana_create_wq_obj() assigns the firmware-returned id. If creation
fails before that, cleanup calls mana_gd_destroy_cq() with id 0, NULLing
gc->cq_table[0] and silently breaking whichever real CQ owns that slot.

Initialize queue->id to INVALID_QUEUE_ID right after allocation, matching
mana_gd_create_eq(). The existing (id >= max_num_cqs) guard then
short-circuits cleanly.

> diff --git a/drivers/net/ethernet/microsoft/mana/gdma_main.c b/drivers/net/ethernet/microsoft/mana/gdma_main.c
> index d8e816882f02c..ac71ca8450bf4 100644
> --- a/drivers/net/ethernet/microsoft/mana/gdma_main.c
> +++ b/drivers/net/ethernet/microsoft/mana/gdma_main.c
> @@ -1192,6 +1192,8 @@ int mana_gd_create_mana_wq_cq(struct gdma_dev *gd,
>  	if (!queue)
>  		return -ENOMEM;
>  
> +	queue->id = INVALID_QUEUE_ID;
> +
>  	gmi = &queue->mem_info;
>  	err = mana_gd_alloc_memory(gc, spec->queue_size, gmi);
>  	if (err) {

[Severity: High]
This is a pre-existing issue, but I noticed mana_gd_create_mana_eq() leaks a
firmware DMA region object on its error path if the EQ hardware creation fails.

In mana_gd_create_mana_eq(), mana_gd_create_dma_region() registers a DMA region
with the hardware and returns a valid handle. If the subsequent call to
mana_gd_create_eq() fails, execution jumps to the out label:

drivers/net/ethernet/microsoft/mana/gdma_main.c:mana_gd_create_mana_eq() {
    ...
	err = mana_gd_create_eq(gd, spec, true, queue);
	if (err)
		goto out;
    ...
out:
	dev_err(gc->dev, "Failed to create queue type %d of size: %u, err: %d\n",
		spec->type, spec->queue_size, err);
	mana_gd_free_memory(gmi);
free_q:
	kfree(queue);
	return err;
}

Does this code need to call mana_gd_destroy_dma_region(gc,
gmi->dma_region_handle) in the error path?

Without it, does this leak the hardware DMA region resource and leave freed
physical pages mapped in the hardware, potentially creating a hardware
use-after-free scenario?

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