Re: [PATCH rdma-next v2] RDMA/bnxt_re: Validate udata before executing commands

[Selvin Xavier]

On Tue, Jul 14, 2026 at 4:20 PM Leon Romanovsky <leon@xxxxxxxxxx> wrote:
>
> From: Leon Romanovsky <leonro@xxxxxxxxxx>
>
> The destroy callbacks currently zero the udata output after tearing down
> driver resources. If the userspace access fails, uverbs preserves the
> uobject and allows the destroy callback to run again, even though the
> driver resource has already been freed.
>
> Call ib_no_udata_io() before teardown so udata failures are detected
> while the resource is still intact, then return success after teardown
> completes.
>
> As part of this change, move ib_respond_empty_udata() to the start of
> the create and modify flows. While this is not strictly required for
> general create flows, as the core layer unwinds uobjects on failure, it
> is necessary for create AH. In _rdma_create_ah(), the HW object is
> otherwise leaked.
>
> Fixes: bed686d8dcd4 ("RDMA/bnxt_re: Use ib_respond_empty_udata()")
> Signed-off-by: Leon Romanovsky <leonro@xxxxxxxxxx>
Acked-by: Selvin Xavier <selvin.xavier@xxxxxxxxxxxx>

> ---
> Jacob, Selvin
>
> I didn't add your tags, as this patch was slightly changed from the
> previous version.
>
> Thanks
>
> Changes in v2:
> - Changed create and modify paths too
> - Link to v1: https://patch.msgid.link/20260713-fix-destroy-no-udata-v1-0-fcca2e34fd57@xxxxxxxxxx
> ---
>  drivers/infiniband/hw/bnxt_re/ib_verbs.c | 65 +++++++++++++++-----------------
>  1 file changed, 30 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/infiniband/hw/bnxt_re/ib_verbs.c b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
> index 90138d64adee..adc693736769 100644
> --- a/drivers/infiniband/hw/bnxt_re/ib_verbs.c
> +++ b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
> @@ -694,7 +694,7 @@ int bnxt_re_dealloc_pd(struct ib_pd *ib_pd, struct ib_udata *udata)
>         struct bnxt_re_dev *rdev = pd->rdev;
>         int ret;
>
> -       ret = ib_is_udata_in_empty(udata);
> +       ret = ib_no_udata_io(udata);
>         if (ret)
>                 return ret;
>
> @@ -711,7 +711,7 @@ int bnxt_re_dealloc_pd(struct ib_pd *ib_pd, struct ib_udata *udata)
>                                            &pd->qplib_pd))
>                         atomic_dec(&rdev->stats.res.pd_count);
>         }
> -       return ib_respond_empty_udata(udata);
> +       return 0;
>  }
>
>  int bnxt_re_alloc_pd(struct ib_pd *ibpd, struct ib_udata *udata)
> @@ -843,7 +843,7 @@ int bnxt_re_create_ah(struct ib_ah *ib_ah, struct rdma_ah_init_attr *init_attr,
>         u8 nw_type;
>         int rc;
>
> -       rc = ib_is_udata_in_empty(udata);
> +       rc = ib_no_udata_io(udata);
>         if (rc)
>                 return rc;
>
> @@ -900,7 +900,7 @@ int bnxt_re_create_ah(struct ib_ah *ib_ah, struct rdma_ah_init_attr *init_attr,
>         if (active_ahs > rdev->stats.res.ah_watermark)
>                 rdev->stats.res.ah_watermark = active_ahs;
>
> -       return ib_respond_empty_udata(udata);
> +       return 0;
>  }
>
>  int bnxt_re_query_ah(struct ib_ah *ib_ah, struct rdma_ah_attr *ah_attr)
> @@ -1014,7 +1014,7 @@ int bnxt_re_destroy_qp(struct ib_qp *ib_qp, struct ib_udata *udata)
>         unsigned int flags;
>         int rc;
>
> -       rc = ib_is_udata_in_empty(udata);
> +       rc = ib_no_udata_io(udata);
>         if (rc)
>                 return rc;
>
> @@ -1063,7 +1063,7 @@ int bnxt_re_destroy_qp(struct ib_qp *ib_qp, struct ib_udata *udata)
>         if (scq_nq != rcq_nq)
>                 bnxt_re_synchronize_nq(rcq_nq);
>
> -       return ib_respond_empty_udata(udata);
> +       return 0;
>  }
>
>  static u8 __from_ib_qp_type(enum ib_qp_type type)
> @@ -2147,7 +2147,7 @@ int bnxt_re_destroy_srq(struct ib_srq *ib_srq, struct ib_udata *udata)
>         struct bnxt_qplib_srq *qplib_srq = &srq->qplib_srq;
>         int ret;
>
> -       ret = ib_is_udata_in_empty(udata);
> +       ret = ib_no_udata_io(udata);
>         if (ret)
>                 return ret;
>
> @@ -2158,7 +2158,7 @@ int bnxt_re_destroy_srq(struct ib_srq *ib_srq, struct ib_udata *udata)
>                 free_page((unsigned long)srq->uctx_srq_page);
>         ib_umem_release(srq->umem);
>         atomic_dec(&rdev->stats.res.srq_count);
> -       return ib_respond_empty_udata(udata);
> +       return 0;
>  }
>
>  static int bnxt_re_init_user_srq(struct bnxt_re_dev *rdev,
> @@ -2296,34 +2296,25 @@ int bnxt_re_modify_srq(struct ib_srq *ib_srq, struct ib_srq_attr *srq_attr,
>  {
>         struct bnxt_re_srq *srq = container_of(ib_srq, struct bnxt_re_srq,
>                                                ib_srq);
> -       struct bnxt_re_dev *rdev = srq->rdev;
>         int ret;
>
> -       ret = ib_is_udata_in_empty(udata);
> +       ret = ib_no_udata_io(udata);
>         if (ret)
>                 return ret;
>
> -       switch (srq_attr_mask) {
> -       case IB_SRQ_MAX_WR:
> -               /* SRQ resize is not supported */
> +       if (srq_attr_mask != IB_SRQ_LIMIT)
>                 return -EINVAL;
> -       case IB_SRQ_LIMIT:
> -               /* Change the SRQ threshold */
> -               if (srq_attr->srq_limit > srq->qplib_srq.max_wqe)
> -                       return -EINVAL;
>
> -               srq->qplib_srq.threshold = srq_attr->srq_limit;
> -               bnxt_qplib_srq_arm_db(&srq->qplib_srq.dbinfo, srq->qplib_srq.threshold);
> -
> -               /* On success, update the shadow */
> -               srq->srq_limit = srq_attr->srq_limit;
> -               /* No need to Build and send response back to udata */
> -               return ib_respond_empty_udata(udata);
> -       default:
> -               ibdev_err(&rdev->ibdev,
> -                         "Unsupported srq_attr_mask 0x%x", srq_attr_mask);
> +       if (srq_attr->srq_limit > srq->qplib_srq.max_wqe)
>                 return -EINVAL;
> -       }
> +
> +       srq->qplib_srq.threshold = srq_attr->srq_limit;
> +       bnxt_qplib_srq_arm_db(&srq->qplib_srq.dbinfo, srq->qplib_srq.threshold);
> +
> +       /* On success, update the shadow */
> +       srq->srq_limit = srq_attr->srq_limit;
> +       /* No need to Build and send response back to udata */
> +       return 0;
>  }
>
>  int bnxt_re_query_srq(struct ib_srq *ib_srq, struct ib_srq_attr *srq_attr)
> @@ -2436,7 +2427,7 @@ int bnxt_re_modify_qp(struct ib_qp *ib_qp, struct ib_qp_attr *qp_attr,
>         unsigned int flags;
>         u8 nw_type;
>
> -       rc = ib_is_udata_in_empty(udata);
> +       rc = ib_no_udata_io(udata);
>         if (rc)
>                 return rc;
>
> @@ -2688,7 +2679,7 @@ int bnxt_re_modify_qp(struct ib_qp *ib_qp, struct ib_qp_attr *qp_attr,
>                 if (rc)
>                         return rc;
>         }
> -       return ib_respond_empty_udata(udata);
> +       return 0;
>  }
>
>  int bnxt_re_query_qp(struct ib_qp *ib_qp, struct ib_qp_attr *qp_attr,
> @@ -3470,7 +3461,7 @@ int bnxt_re_destroy_cq(struct ib_cq *ib_cq, struct ib_udata *udata)
>         nq = cq->qplib_cq.nq;
>         cctx = rdev->chip_ctx;
>
> -       ret = ib_is_udata_in_empty(udata);
> +       ret = ib_no_udata_io(udata);
>         if (ret)
>                 return ret;
>
> @@ -3485,7 +3476,7 @@ int bnxt_re_destroy_cq(struct ib_cq *ib_cq, struct ib_udata *udata)
>         atomic_dec(&rdev->stats.res.cq_count);
>         kfree(cq->cql);
>         ib_umem_release(cq->umem);
> -       return ib_respond_empty_udata(udata);
> +       return 0;
>  }
>
>  int bnxt_re_create_user_cq(struct ib_cq *ibcq, const struct ib_cq_init_attr *attr,
> @@ -3687,6 +3678,10 @@ int bnxt_re_resize_cq(struct ib_cq *ibcq, unsigned int cqe,
>         if (rc)
>                 goto fail;
>
> +       rc = ib_respond_empty_udata(udata);
> +       if (rc)
> +               goto fail;
> +
>         cq->resize_umem = ib_umem_get_va(&rdev->ibdev, req.cq_va,
>                                          entries * sizeof(struct cq_base),
>                                          IB_ACCESS_LOCAL_WRITE);
> @@ -3716,7 +3711,7 @@ int bnxt_re_resize_cq(struct ib_cq *ibcq, unsigned int cqe,
>         cq->ib_cq.cqe = cq->resize_cqe;
>         atomic_inc(&rdev->stats.res.resize_count);
>
> -       return ib_respond_empty_udata(udata);
> +       return 0;
>
>  fail:
>         if (cq->resize_umem) {
> @@ -4448,7 +4443,7 @@ int bnxt_re_dereg_mr(struct ib_mr *ib_mr, struct ib_udata *udata)
>         struct bnxt_re_dev *rdev = mr->rdev;
>         int rc;
>
> -       rc = ib_is_udata_in_empty(udata);
> +       rc = ib_no_udata_io(udata);
>         if (rc)
>                 return rc;
>
> @@ -4471,7 +4466,7 @@ int bnxt_re_dereg_mr(struct ib_mr *ib_mr, struct ib_udata *udata)
>         atomic_dec(&rdev->stats.res.mr_count);
>         if (rc)
>                 return rc;
> -       return ib_respond_empty_udata(udata);
> +       return 0;
>  }
>
>  static int bnxt_re_set_page(struct ib_mr *ib_mr, u64 addr)
>
> ---
> base-commit: eeb9697db6c16d9bb2ce7b7ddf95aa20305aa9f2
> change-id: 20260712-fix-destroy-no-udata-dfa990b985ea
>
> Best regards,
> --
> Leon Romanovsky <leon@xxxxxxxxxx>
>

Attachment: smime.p7s
Description: S/MIME Cryptographic Signature