Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] Incomplete fix for CQ ID recycle window on Ethernet and RDMA kernel CQs -- --- Patch [1]: [PATCH net v7 1/7] net: mana: reference-count CQs looked up from the EQ handler --- commit 26834a69bfec0a9b46df2c9aa1aa0f248272df13 Author: Long Li <[email protected]> net: mana: reference-count CQs looked up from the EQ handler This patch introduces reference counting for CQs looked up from the EQ handler in the MANA driver to prevent a use-after-free. It also reorders the RDMA teardown path to clear the dispatch entry before destroying the hardware CQ, aiming to close a window where a recycled cq_id could alias the outgoing entry. > diff --git a/drivers/infiniband/hw/mana/cq.c b/drivers/infiniband/hw/mana/cq.c > index f2547989f4229..022c82479ef6c 100644 > --- a/drivers/infiniband/hw/mana/cq.c > +++ b/drivers/infiniband/hw/mana/cq.c > @@ -108,11 +108,12 @@ int mana_ib_destroy_cq(struct ib_cq *ibcq, struct > ib_udata *udata) > > mdev = container_of(ibdev, struct mana_ib_dev, ib_dev); > > + /* Detach the dispatch entry first, then stop the HW CQ and free the > + * queue. A completion racing teardown then finds an empty slot, and > + * a recycled cq_id cannot alias this CQ. Errors are logged inside. > + */ > mana_ib_remove_cq_cb(mdev, cq); > > - /* Ignore return code as there is not much we can do about it. > - * The error message is printed inside. > - */ > mana_ib_gd_destroy_cq(mdev, cq); > > mana_ib_destroy_queue(mdev, &cq->queue); [ ... ] > @@ -149,23 +146,41 @@ int mana_ib_install_cq_cb(struct mana_ib_dev *mdev, > struct mana_ib_cq *cq) > void mana_ib_remove_cq_cb(struct mana_ib_dev *mdev, struct mana_ib_cq *cq) > { > struct gdma_context *gc = mdev_to_gc(mdev); > + struct gdma_queue __rcu **cq_table; > + struct gdma_queue *gdma_cq; > > - if (cq->queue.id >= gc->max_num_cqs || cq->queue.id == INVALID_QUEUE_ID) > + if (cq->queue.id == INVALID_QUEUE_ID || cq->queue.id >= gc->max_num_cqs) > return; > > if (cq->queue.kmem) > /* Then it will be cleaned and removed by the mana */ > return; [Severity: High] This is a pre-existing issue, but does this early return leave the CQ ID recycle window open for RDMA kernel CQs? For kernel CQs, mana_ib_remove_cq_cb() returns early, bypassing the software CQ unpublish step that was explicitly added to close the recycle window. Because it skips this, the hardware CQ is destroyed via mana_ib_gd_destroy_cq() before the queue is torn down. If the hardware immediately frees the cq_id, a concurrent thread could create a new CQ, be assigned the same cq_id, and fail with -EINVAL in mana_gd_publish_cq() because the old CQ is still in gc->cq_table. Also, does this same race exist in the Ethernet paths? Looking at mana_destroy_rxq() in drivers/net/ethernet/microsoft/mana/mana_en.c, the hardware object is destroyed before mana_deinit_cq() removes the old software CQ from the dispatch table: mana_destroy_rxq() { ... if (rxq->rxobj != INVALID_MANA_HANDLE) mana_destroy_wq_obj(apc, GDMA_RQ, rxq->rxobj); mana_deinit_cq(apc, &rxq->rx_cq); ... } A similar inverted destruction sequence seems to exist in mana_destroy_txq(). Could this lead to spurious queue creation failures and permanently detached network interfaces after operations like MTU changes? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
