Re: [PATCH rdma-next v2 3/6] RDMA/irdma: Support UMEM attribute for QP creation

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] rdma: resource leak of qp_num in irdma_create_qp() error path
--

commit c4616d335368961fe171e368fb4fc4f2e2fa096d
Author: David Hu <xuehaohu@google.com>
RDMA/irdma: Support UMEM attribute for QP creation

This commit introduces support for the standard 1-step queue buffer creation
model for QPs by using UVERBS_ATTR_CREATE_QP_BUF_UMEM and ib_umem_get_attr().
It allocates the backing MR and programs the hardware pble directly from the
pinned umem, adding cleanup routines to irdma_free_qp_rsrc() upon destruction.

> diff --git a/drivers/infiniband/hw/irdma/verbs.c b/drivers/infiniband/hw/irdma/verbs.c
> index b2ffdc8e3d677..ab25445f38282 100644
> --- a/drivers/infiniband/hw/irdma/verbs.c
> +++ b/drivers/infiniband/hw/irdma/verbs.c
[ ... ]
> @@ -713,6 +708,52 @@ static int irdma_setup_umode_qp(struct irdma_ucontext *ucontext,
>  		ukinfo->rq_size = ukinfo->rq_depth >> ukinfo->rq_shift;
>  	}
>  
> +	if (attrs && uverbs_attr_is_valid(attrs, UVERBS_ATTR_CREATE_QP_BUF_UMEM)) {
> +		struct irdma_mem_reg_req qp_req = {};
> +		struct ib_umem *umem;
> +		u32 total;
> +
> +		/* Legacy context does not provide depth info */
> +		if (!ucontext->use_raw_attrs)
> +			return -EOPNOTSUPP;
> +
> +		qp_req.sq_pages = DIV_ROUND_UP(ukinfo->sq_depth * IRDMA_QP_WQE_MIN_SIZE,
> +					       IRDMA_HW_PAGE_SIZE);
> +		/* In case srq is used, this will be a single page */
> +		qp_req.rq_pages = DIV_ROUND_UP(ukinfo->rq_depth * IRDMA_QP_WQE_MIN_SIZE,
> +					       IRDMA_HW_PAGE_SIZE);
> +		total = qp_req.sq_pages + qp_req.rq_pages + IRDMA_SHADOW_PGCNT;
> +
> +		umem = ib_umem_get_attr(&iwdev->ibdev, attrs,
> +					UVERBS_ATTR_CREATE_QP_BUF_UMEM,
> +					total << IRDMA_HW_PAGE_SHIFT,
> +					IB_ACCESS_LOCAL_WRITE);
> +		if (IS_ERR(umem))
> +			return PTR_ERR(umem);
> +
> +		iwqp->iwpbl = irdma_setup_q_pbl_from_umem(iwdev, umem,
> +							  iwqp->ibqp.pd,
> +							  IRDMA_MEMREG_TYPE_QP,
> +							  &qp_req);
> +		if (IS_ERR(iwqp->iwpbl)) {
> +			ret = PTR_ERR(iwqp->iwpbl);
> +			iwqp->iwpbl = NULL;
> +			ib_umem_release(umem);
> +			return ret;
> +		}

[Severity: Medium]
If the newly added UMEM QP setup fails and returns an error here,
irdma_create_qp() will jump to its error path:

drivers/infiniband/hw/irdma/verbs.c:irdma_create_qp() {
    ...
    if (udata) {
        ...
        err_code = irdma_setup_umode_qp(ucontext, &ureq, iwdev, iwqp,
                                        &init_info, init_attr, attrs);
    } else {
    ...
    if (err_code) {
        ibdev_dbg(&iwdev->ibdev, "VERBS: setup qp failed\n");
        goto error;
    }
    ...
error:
    irdma_free_qp_rsrc(iwqp);
    return err_code;
}

However, at this point irdma_sc_qp_init() has not yet been executed, so
iwqp->sc_qp.qp_uk.qp_id is still uninitialized (0).

Does this cause irdma_free_qp_rsrc() to read a 0 value for qp_num and bypass
the deallocation logic (if (qp_num > 2)), permanently leaking the allocated
QP ID from the rf->allocated_qps bitmap?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261015011940.4122452-1-dhu@x6u.co?part=3