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 <[email protected]> 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/[email protected]?part=1
