> From: Leon Romanovsky <[email protected]> > > mana_table_store_ud_qp() publishes a QP at its send-queue id before > inserting the receive-queue id, dropping the XArray lock between the two > xa_insert_irq() calls. A concurrent completion handler can look up the QP and > take a transient reference. When the second insertion fails, the rollback > erased only the send-queue entry and returned, leaving both the initial table > reference and the transient reference outstanding while RDMA core frees the > QP, causing a use-after-free. > > Drain the reference as normal destruction does: drop the initial reference and > wait for qp->free, releasing the QP only after every concurrent lookup returns > its reference. > > Fixes: 8001e9257eca ("RDMA/mana_ib: extend mana QP table") > Signed-off-by: Leon Romanovsky <[email protected]>
Reviewed-by: Konstantin Taranov <[email protected]> > --- > Konstantin, > > I saw this in one of the Sashiko runs, and I believe the AI is correct. The > reference can be acquired through the GDMA_EQE_RNIC_QP_FATAL event. > I doubt that it is possible for EQEs. The eqe only can mention an RQ and we first try to insert an SQ. As well as when we drain GSI QP, we also look up with an RQ. I am also trying to think whether CQE with SQ QID can cause it, which would reveal a HW bug/race. But I do agree that the patch makes sense and we should have it, regardless of whether it is possible. Thanks a lot for the patch. I have sent recently a UC-related patch that changes the same lines. I will rebase it on top of your change. > Thanks > --- > drivers/infiniband/hw/mana/qp.c | 10 ++++++++-- > 1 file changed, 8 insertions(+), 2 deletions(-) > > diff --git a/drivers/infiniband/hw/mana/qp.c > b/drivers/infiniband/hw/mana/qp.c index b5ff07e34eb7..c52f3ec14032 > 100644 > --- a/drivers/infiniband/hw/mana/qp.c > +++ b/drivers/infiniband/hw/mana/qp.c > @@ -472,6 +472,12 @@ static void mana_table_remove_rc_qp(struct > mana_ib_dev *mdev, struct mana_ib_qp > xa_erase_irq(&mdev->qp_table_wq, qp->ibqp.qp_num); } > > +static void mana_table_drain_qp_ref(struct mana_ib_qp *qp) { > + mana_put_qp_ref(qp); > + wait_for_completion(&qp->free); > +} > + > static int mana_table_store_ud_qp(struct mana_ib_dev *mdev, struct > mana_ib_qp *qp) { > u32 qids = qp->ud_qp.queues[MANA_UD_SEND_QUEUE].id | > MANA_SENDQ_MASK; @@ -490,6 +496,7 @@ static int > mana_table_store_ud_qp(struct mana_ib_dev *mdev, struct mana_ib_qp *q > > remove_sq: > xa_erase_irq(&mdev->qp_table_wq, qids); > + mana_table_drain_qp_ref(qp); > return err; > } > > @@ -537,8 +544,7 @@ static void mana_table_remove_qp(struct > mana_ib_dev *mdev, > qp->ibqp.qp_type); > return; > } > - mana_put_qp_ref(qp); > - wait_for_completion(&qp->free); > + mana_table_drain_qp_ref(qp); > } > > static int mana_ib_create_rc_qp(struct ib_qp *ibqp, struct ib_pd *ibpd, > > --- > base-commit: 74f49255492a62658f36bf2578d7916f1c6ffad1 > change-id: 20260721-if-mana-table-store-qp-qids-partiall-afa14f023261 > > Best regards, > -- > Leon Romanovsky <[email protected]>
