Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] In `qedr_create_user_qp`, the error code from `qedr_copy_qp_uresp` is overwritten by the return value of `rdma_destroy_qp` in the error cleanup path, causing a false success return. --
commit 5dc8fd66d59366118ca19c878df5216ffd4a4a52 Author: Jason Gunthorpe <[email protected]> RDMA/qedr: Replace qedr_ib_copy_to_udata() with ib_respond_udata() This replaces qedr_ib_copy_to_udata() with ib_respond_udata() to handle copying data to user-space, removing the local inline wrapper. > diff --git a/drivers/infiniband/hw/qedr/verbs.c > b/drivers/infiniband/hw/qedr/verbs.c > index 3b86ea1cf8888..79190c5b8b50b 100644 > --- a/drivers/infiniband/hw/qedr/verbs.c > +++ b/drivers/infiniband/hw/qedr/verbs.c [ ... ] > @@ -1311,13 +1294,7 @@ static int qedr_copy_qp_uresp(struct qedr_dev *dev, > uresp->atomic_supported = dev->atomic_cap != IB_ATOMIC_NONE; > uresp->qp_id = qp->qp_id; > > - rc = qedr_ib_copy_to_udata(udata, uresp, sizeof(*uresp)); > - if (rc) > - DP_ERR(dev, > - "create qp: failed a copy to user space with qp > icid=0x%x.\n", > - qp->icid); > - > - return rc; > + return ib_respond_udata(udata, *uresp); This isn't a bug introduced by your patch, but does the error handling in qedr_create_user_qp() overwrite the return value of this function? If qedr_copy_qp_uresp() or subsequent qedr_db_recovery_add() calls fail, the function branches to the err label: drivers/infiniband/hw/qedr/verbs.c:qedr_create_user_qp() { ... if (udata) { rc = qedr_copy_qp_uresp(dev, qp, udata, &uresp); if (rc) goto err; } ... err: rc = dev->ops->rdma_destroy_qp(dev->rdma_ctx, qp->qed_qp); if (rc) DP_ERR(dev, "create qp: fatal fault. rc=%d", rc); err1: qedr_cleanup_user(dev, ctx, qp); return rc; } Since destroying the newly allocated QP typically succeeds and returns 0, would this unconditionally overwrite the original error code in rc, returning a false success to user-space? If user-space subsequently attempts to post work requests to this destroyed QP, could it lead to hardware undefined behavior, PCIe errors, or system crashes? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=6
