Re: [PATCH rdma-next v3 5/7] RDMA/irdma: Use robust udata helper for QP creation

[Leon Romanovsky <leon@kernel.org>]

On Thu, Jul 02, 2026 at 05:06:50PM +0000, Jacob Moroni wrote:
> Replace the manual udata input copy and validation during
> QP creation with the robust helper.
> 
> The irdma driver is backwards compatible with the legacy
> i40iw userspace provider. The current create_qp ABI contains
> two 8 byte fields. The legacy i40iw ABI was the same but
> also contained two additional fields which were never actually
> used. Furthermore, the i40iw userspace provider never explicitly
> zero-initialized those extra fields, so there is a chance that
> existing binaries are passing non-zero garbage values down
> to the kernel.
> 
> Previously, the irdma driver only copied out the first 16
> bytes and did not have any check for the rest of the buffer
> being zero, so that additional garbage didn't matter.
> 
> By switching to ib_copy_validate_udata_in(), we will now be
> checking to ensure that data beyond the kernel's definition
> of the request is all zero.
> 
> In order to avoid breaking legacy binaries, we therefore need
> to increase the request structure size to cover those garbage
> fields.
> 
> - Legacy binaries will continue to pass down a 32 byte request,
>   with the driver copying the entire 32 bytes out but ignoring
>   the second 16 bytes, just as before.
> 
> - Newer binaries will pass down the normal 16 byte request. The
>   ib_copy_validate_udata_in() call will allow this to succeed
>   because we use user_compl_ctx as our minimum length (16 bytes).
> 
> - If the request is ever extended, the new fields would be
>   added after the "don't use" fields and would work as per
>   the normal uAPI mechanism.
> 
> Signed-off-by: Jacob Moroni <jmoroni@google.com>
> ---
>  drivers/infiniband/hw/irdma/verbs.c | 41 ++++++++++++++++-------------
>  include/uapi/rdma/irdma-abi.h       |  1 +
>  2 files changed, 23 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/infiniband/hw/irdma/verbs.c b/drivers/infiniband/hw/irdma/verbs.c
> index 519ac780e9f1..00a648922c7b 100644
> --- a/drivers/infiniband/hw/irdma/verbs.c
> +++ b/drivers/infiniband/hw/irdma/verbs.c
> @@ -632,37 +632,29 @@ static void irdma_setup_virt_qp(struct irdma_device *iwdev,
>  
>  /**
>   * irdma_setup_umode_qp - setup sq and rq size in user mode qp
> - * @udata: udata
> + * @ucontext: user context
> + * @req: user request pointer
>   * @iwdev: iwarp device
>   * @iwqp: qp ptr (user or kernel)
>   * @info: initialize info to return
>   * @init_attr: Initial QP create attributes
>   */
> -static int irdma_setup_umode_qp(struct ib_udata *udata,
> +static int irdma_setup_umode_qp(struct irdma_ucontext *ucontext,
> +				struct irdma_create_qp_req *req,
>  				struct irdma_device *iwdev,
>  				struct irdma_qp *iwqp,
>  				struct irdma_qp_init_info *info,
>  				struct ib_qp_init_attr *init_attr)
>  {
> -	struct irdma_ucontext *ucontext = rdma_udata_to_drv_context(udata,
> -				struct irdma_ucontext, ibucontext);
>  	struct irdma_qp_uk_init_info *ukinfo = &info->qp_uk_init_info;
> -	struct irdma_create_qp_req req;
>  	unsigned long flags;
>  	int ret;
>  
> -	ret = ib_copy_from_udata(&req, udata,
> -				 min(sizeof(req), udata->inlen));
> -	if (ret) {
> -		ibdev_dbg(&iwdev->ibdev, "VERBS: ib_copy_from_data fail\n");
> -		return ret;
> -	}
> -
> -	iwqp->ctx_info.qp_compl_ctx = req.user_compl_ctx;
> +	iwqp->ctx_info.qp_compl_ctx = req->user_compl_ctx;
>  	iwqp->user_mode = 1;
>  
>  	spin_lock_irqsave(&ucontext->qp_reg_mem_list_lock, flags);
> -	iwqp->iwpbl = irdma_get_pbl((unsigned long)req.user_wqe_bufs,
> +	iwqp->iwpbl = irdma_get_pbl((unsigned long)req->user_wqe_bufs,
>  				    &ucontext->qp_reg_mem_list);
>  	spin_unlock_irqrestore(&ucontext->qp_reg_mem_list_lock, flags);

Which codebase did you use for this series?

That function differs from the current implementation, which has
remained unchanged since 2023:
https://git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma.git/tree/drivers/infiniband/hw/irdma/verbs.c#n639

Thanks