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

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

[Leon Romanovsky]

From: Leon Romanovsky <leonro@nvidia.com>

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@nvidia.com>
---
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@nvidia.com
---
 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@kernel.org>

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

[Selvin Xavier]

[-- Attachment #1: Type: text/plain, Size: 9198 bytes --]

On Tue, Jul 14, 2026 at 4:20 PM Leon Romanovsky <leon@kernel.org> wrote:
>
> From: Leon Romanovsky <leonro@nvidia.com>
>
> 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@nvidia.com>
Acked-by: Selvin Xavier <selvin.xavier@broadcom.com>

> ---
> 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@nvidia.com
> ---
>  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@kernel.org>
>

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5473 bytes --]

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

[Leon Romanovsky]

On Tue, 14 Jul 2026 13:50:00 +0300, Leon Romanovsky wrote:
> 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.
> 
> [...]

Applied, thanks!

[1/1] RDMA/bnxt_re: Validate udata before executing commands
      https://git.kernel.org/rdma/rdma/c/36a29408795fe2

Best regards,
-- 
Leon Romanovsky <leon@kernel.org>