Re: [PATCH v3 06/10] RDMA/qedr: Replace qedr_ib_copy_to_udata() with ib_respond_udata()

[sashiko-bot@kernel.org]

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 <jgg@ziepe.ca>

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/0-v3-4effdebad75a+e1-rdma_udata_rep_jgg@nvidia.com?part=6