[PATCH rdma v1 0/1] IB/core: Fix input len in multiple user verbs

[PATCH rdma v1 0/1] IB/core: Fix input len in multiple user verbs

[Ram Amrani]

This patch follows [1] that fixed three verbs and extends that fix to
the other verbs as well.

This fix is required for qedr to support existing and future libraries.

I was able to test the fix only partially due to its scope hence careful
review is required, and testing, of course.

v0->v1:
 * Updated vendor verbs to match the core changes.


[1] https://www.spinics.net/lists/linux-rdma/msg33405.html 

Ram Amrani (1):
  IB/core: Fix input len in multiple user verbs

 drivers/infiniband/core/uverbs_cmd.c         | 70 ++++++++++++++++------------
 drivers/infiniband/hw/mlx5/cq.c              |  6 +--
 drivers/infiniband/hw/mlx5/main.c            | 11 ++---
 drivers/infiniband/hw/mthca/mthca_provider.c |  2 +-
 4 files changed, 46 insertions(+), 43 deletions(-)

-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH rdma v1 1/1] IB/core: Fix input len in multiple user verbs

[Ram Amrani]

Most user verbs pass user data to the kernel with the inclusion of the
ib_uverbs_cmd_hdr structure. This is problematic because the vendor has
no ideas if the verb was called by a legacy verb or an extended verb.
Also, the incosistency between the verbs is confusing.

Fixes: 565197dd8fb1 ("IB/core: Extend ib_uverbs_create_cq")
Signed-off-by: Ram Amrani <Ram.Amrani-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
Signed-off-by: Ariel Elior <Ariel.Elior-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
---
 drivers/infiniband/core/uverbs_cmd.c         | 70 ++++++++++++++++------------
 drivers/infiniband/hw/mlx5/cq.c              |  6 +--
 drivers/infiniband/hw/mlx5/main.c            | 11 ++---
 drivers/infiniband/hw/mthca/mthca_provider.c |  2 +-
 4 files changed, 46 insertions(+), 43 deletions(-)

diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
index 70b7fb1..c418a0a 100644
--- a/drivers/infiniband/core/uverbs_cmd.c
+++ b/drivers/infiniband/core/uverbs_cmd.c
@@ -91,9 +91,10 @@ ssize_t ib_uverbs_get_context(struct ib_uverbs_file *file,
 		goto err;
 	}
 
-	INIT_UDATA(&udata, buf + sizeof cmd,
-		   (unsigned long) cmd.response + sizeof resp,
-		   in_len - sizeof cmd, out_len - sizeof resp);
+	INIT_UDATA(&udata, buf + sizeof(cmd),
+		   (unsigned long) cmd.response + sizeof(resp),
+		   in_len - sizeof(cmd) - sizeof(struct ib_uverbs_cmd_hdr),
+		   out_len - sizeof(resp));
 
 	ret = ib_rdmacg_try_charge(&cg_obj, ib_dev, RDMACG_RESOURCE_HCA_HANDLE);
 	if (ret)
@@ -313,9 +314,10 @@ ssize_t ib_uverbs_alloc_pd(struct ib_uverbs_file *file,
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
-	INIT_UDATA(&udata, buf + sizeof cmd,
-		   (unsigned long) cmd.response + sizeof resp,
-		   in_len - sizeof cmd, out_len - sizeof resp);
+	INIT_UDATA(&udata, buf + sizeof(cmd),
+		   (unsigned long) cmd.response + sizeof(resp),
+                   in_len - sizeof(cmd) - sizeof(struct ib_uverbs_cmd_hdr),
+                   out_len - sizeof(resp));
 
 	uobj  = uobj_alloc(uobj_get_type(pd), file->ucontext);
 	if (IS_ERR(uobj))
@@ -482,9 +484,10 @@ ssize_t ib_uverbs_open_xrcd(struct ib_uverbs_file *file,
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
-	INIT_UDATA(&udata, buf + sizeof cmd,
-		   (unsigned long) cmd.response + sizeof resp,
-		   in_len - sizeof cmd, out_len - sizeof  resp);
+	INIT_UDATA(&udata, buf + sizeof(cmd),
+		   (unsigned long) cmd.response + sizeof(resp),
+                   in_len - sizeof(cmd) - sizeof(struct ib_uverbs_cmd_hdr),
+                   out_len - sizeof(resp));
 
 	mutex_lock(&file->device->xrcd_tree_mutex);
 
@@ -646,9 +649,10 @@ ssize_t ib_uverbs_reg_mr(struct ib_uverbs_file *file,
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
-	INIT_UDATA(&udata, buf + sizeof cmd,
-		   (unsigned long) cmd.response + sizeof resp,
-		   in_len - sizeof cmd, out_len - sizeof resp);
+	INIT_UDATA(&udata, buf + sizeof(cmd),
+		   (unsigned long) cmd.response + sizeof(resp),
+                   in_len - sizeof(cmd) - sizeof(struct ib_uverbs_cmd_hdr),
+                   out_len - sizeof(resp));
 
 	if ((cmd.start & ~PAGE_MASK) != (cmd.hca_va & ~PAGE_MASK))
 		return -EINVAL;
@@ -740,7 +744,8 @@ ssize_t ib_uverbs_rereg_mr(struct ib_uverbs_file *file,
 
 	INIT_UDATA(&udata, buf + sizeof(cmd),
 		   (unsigned long) cmd.response + sizeof(resp),
-		   in_len - sizeof(cmd), out_len - sizeof(resp));
+                   in_len - sizeof(cmd) - sizeof(struct ib_uverbs_cmd_hdr),
+                   out_len - sizeof(resp));
 
 	if (cmd.flags & ~IB_MR_REREG_SUPPORTED || !cmd.flags)
 		return -EINVAL;
@@ -1080,7 +1085,8 @@ ssize_t ib_uverbs_create_cq(struct ib_uverbs_file *file,
 
 	INIT_UDATA(&uhw, buf + sizeof(cmd),
 		   (unsigned long)cmd.response + sizeof(resp),
-		   in_len - sizeof(cmd), out_len - sizeof(resp));
+		   in_len - sizeof(cmd) - sizeof(struct ib_uverbs_cmd_hdr),
+		   out_len - sizeof(resp));
 
 	memset(&cmd_ex, 0, sizeof(cmd_ex));
 	cmd_ex.user_handle = cmd.user_handle;
@@ -1161,9 +1167,10 @@ ssize_t ib_uverbs_resize_cq(struct ib_uverbs_file *file,
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
-	INIT_UDATA(&udata, buf + sizeof cmd,
-		   (unsigned long) cmd.response + sizeof resp,
-		   in_len - sizeof cmd, out_len - sizeof resp);
+	INIT_UDATA(&udata, buf + sizeof(cmd),
+		   (unsigned long) cmd.response + sizeof(resp),
+		   in_len - sizeof(cmd) - sizeof(struct ib_uverbs_cmd_hdr),
+		   out_len - sizeof(resp));
 
 	cq = uobj_get_obj_read(cq, cmd.cq_handle, file->ucontext);
 	if (!cq)
@@ -1719,9 +1726,10 @@ ssize_t ib_uverbs_open_qp(struct ib_uverbs_file *file,
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
-	INIT_UDATA(&udata, buf + sizeof cmd,
-		   (unsigned long) cmd.response + sizeof resp,
-		   in_len - sizeof cmd, out_len - sizeof resp);
+	INIT_UDATA(&udata, buf + sizeof(cmd),
+		   (unsigned long) cmd.response + sizeof(resp),
+		   in_len - sizeof(cmd) - sizeof(struct ib_uverbs_cmd_hdr),
+		   out_len - sizeof(resp));
 
 	obj  = (struct ib_uqp_object *)uobj_alloc(uobj_get_type(qp),
 						  file->ucontext);
@@ -2038,7 +2046,8 @@ ssize_t ib_uverbs_modify_qp(struct ib_uverbs_file *file,
 		return -EOPNOTSUPP;
 
 	INIT_UDATA(&udata, buf + sizeof(cmd.base), NULL,
-		   in_len - sizeof(cmd.base), out_len);
+		   in_len - sizeof(cmd.base) - sizeof(struct ib_uverbs_cmd_hdr),
+		   out_len);
 
 	ret = modify_qp(file, &cmd, &udata);
 	if (ret)
@@ -2543,7 +2552,8 @@ ssize_t ib_uverbs_create_ah(struct ib_uverbs_file *file,
 
 	INIT_UDATA(&udata, buf + sizeof(cmd),
 		   (unsigned long)cmd.response + sizeof(resp),
-		   in_len - sizeof(cmd), out_len - sizeof(resp));
+		   in_len - sizeof(cmd) - sizeof(struct ib_uverbs_cmd_hdr),
+		   out_len - sizeof(resp));
 
 	uobj  = uobj_alloc(uobj_get_type(ah), file->ucontext);
 	if (IS_ERR(uobj))
@@ -3609,10 +3619,10 @@ ssize_t ib_uverbs_create_srq(struct ib_uverbs_file *file,
 	xcmd.max_sge	 = cmd.max_sge;
 	xcmd.srq_limit	 = cmd.srq_limit;
 
-	INIT_UDATA(&udata, buf + sizeof cmd,
-		   (unsigned long) cmd.response + sizeof resp,
-		   in_len - sizeof cmd - sizeof(struct ib_uverbs_cmd_hdr),
-		   out_len - sizeof resp);
+	INIT_UDATA(&udata, buf + sizeof(cmd),
+		   (unsigned long) cmd.response + sizeof(resp),
+		   in_len - sizeof(cmd) - sizeof(struct ib_uverbs_cmd_hdr),
+		   out_len - sizeof(resp));
 
 	ret = __uverbs_create_xsrq(file, ib_dev, &xcmd, &udata);
 	if (ret)
@@ -3636,10 +3646,10 @@ ssize_t ib_uverbs_create_xsrq(struct ib_uverbs_file *file,
 	if (copy_from_user(&cmd, buf, sizeof cmd))
 		return -EFAULT;
 
-	INIT_UDATA(&udata, buf + sizeof cmd,
-		   (unsigned long) cmd.response + sizeof resp,
-		   in_len - sizeof cmd - sizeof(struct ib_uverbs_cmd_hdr),
-		   out_len - sizeof resp);
+	INIT_UDATA(&udata, buf + sizeof(cmd),
+		   (unsigned long) cmd.response + sizeof(resp),
+		   in_len - sizeof(cmd) - sizeof(struct ib_uverbs_cmd_hdr),
+		   out_len - sizeof(resp));
 
 	ret = __uverbs_create_xsrq(file, ib_dev, &cmd, &udata);
 	if (ret)
diff --git a/drivers/infiniband/hw/mlx5/cq.c b/drivers/infiniband/hw/mlx5/cq.c
index 94c049b..619e265 100644
--- a/drivers/infiniband/hw/mlx5/cq.c
+++ b/drivers/infiniband/hw/mlx5/cq.c
@@ -751,10 +751,8 @@ static int create_cq_user(struct mlx5_ib_dev *dev, struct ib_udata *udata,
 	void *cqc;
 	int err;
 
-	ucmdlen =
-		(udata->inlen - sizeof(struct ib_uverbs_cmd_hdr) <
-		 sizeof(ucmd)) ? (sizeof(ucmd) -
-				  sizeof(ucmd.reserved)) : sizeof(ucmd);
+	ucmdlen = udata->inlen < sizeof(ucmd) ?
+		  (sizeof(ucmd) - sizeof(ucmd.reserved)) : sizeof(ucmd);
 
 	if (ib_copy_from_udata(&ucmd, udata, ucmdlen))
 		return -EFAULT;
diff --git a/drivers/infiniband/hw/mlx5/main.c b/drivers/infiniband/hw/mlx5/main.c
index 9ecc089..906baf0 100644
--- a/drivers/infiniband/hw/mlx5/main.c
+++ b/drivers/infiniband/hw/mlx5/main.c
@@ -1211,7 +1211,6 @@ static struct ib_ucontext *mlx5_ib_alloc_ucontext(struct ib_device *ibdev,
 	struct mlx5_bfreg_info *bfregi;
 	int ver;
 	int err;
-	size_t reqlen;
 	size_t min_req_v2 = offsetof(struct mlx5_ib_alloc_ucontext_req_v2,
 				     max_cqe_version);
 	bool lib_uar_4k;
@@ -1219,18 +1218,14 @@ static struct ib_ucontext *mlx5_ib_alloc_ucontext(struct ib_device *ibdev,
 	if (!dev->ib_active)
 		return ERR_PTR(-EAGAIN);
 
-	if (udata->inlen < sizeof(struct ib_uverbs_cmd_hdr))
-		return ERR_PTR(-EINVAL);
-
-	reqlen = udata->inlen - sizeof(struct ib_uverbs_cmd_hdr);
-	if (reqlen == sizeof(struct mlx5_ib_alloc_ucontext_req))
+	if (udata->inlen == sizeof(struct mlx5_ib_alloc_ucontext_req))
 		ver = 0;
-	else if (reqlen >= min_req_v2)
+	else if (udata->inlen >= min_req_v2)
 		ver = 2;
 	else
 		return ERR_PTR(-EINVAL);
 
-	err = ib_copy_from_udata(&req, udata, min(reqlen, sizeof(req)));
+	err = ib_copy_from_udata(&req, udata, min(udata->inlen, sizeof(req)));
 	if (err)
 		return ERR_PTR(err);
 
diff --git a/drivers/infiniband/hw/mthca/mthca_provider.c b/drivers/infiniband/hw/mthca/mthca_provider.c
index c197cd9..1a222dc 100644
--- a/drivers/infiniband/hw/mthca/mthca_provider.c
+++ b/drivers/infiniband/hw/mthca/mthca_provider.c
@@ -914,7 +914,7 @@ static struct ib_mr *mthca_reg_user_mr(struct ib_pd *pd, u64 start, u64 length,
 	int err = 0;
 	int write_mtt_size;
 
-	if (udata->inlen - sizeof (struct ib_uverbs_cmd_hdr) < sizeof ucmd) {
+	if (udata->inlen < sizeof ucmd) {
 		if (!to_mucontext(pd->uobject->context)->reg_mr_warned) {
 			mthca_warn(dev, "Process '%s' did not pass in MR attrs.\n",
 				   current->comm);
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma v1 1/1] IB/core: Fix input len in multiple user verbs

[Leon Romanovsky]

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

On Tue, Jun 27, 2017 at 05:04:42PM +0300, Ram Amrani wrote:
> Most user verbs pass user data to the kernel with the inclusion of the
> ib_uverbs_cmd_hdr structure. This is problematic because the vendor has
> no ideas if the verb was called by a legacy verb or an extended verb.

Why vendor should know about it? It has midlayer (ib/core) between him
and user to handle it.

> Also, the incosistency between the verbs is confusing.
>
> Fixes: 565197dd8fb1 ("IB/core: Extend ib_uverbs_create_cq")

It is not related to changes in create_mr and create_qp.

> Signed-off-by: Ram Amrani <Ram.Amrani-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
> Signed-off-by: Ariel Elior <Ariel.Elior-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
> ---
>  drivers/infiniband/core/uverbs_cmd.c         | 70 ++++++++++++++++------------
>  drivers/infiniband/hw/mlx5/cq.c              |  6 +--
>  drivers/infiniband/hw/mlx5/main.c            | 11 ++---
>  drivers/infiniband/hw/mthca/mthca_provider.c |  2 +-
>  4 files changed, 46 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
> index 70b7fb1..c418a0a 100644
> --- a/drivers/infiniband/core/uverbs_cmd.c
> +++ b/drivers/infiniband/core/uverbs_cmd.c
> @@ -91,9 +91,10 @@ ssize_t ib_uverbs_get_context(struct ib_uverbs_file *file,
>  		goto err;
>  	}
>
> -	INIT_UDATA(&udata, buf + sizeof cmd,
> -		   (unsigned long) cmd.response + sizeof resp,
> -		   in_len - sizeof cmd, out_len - sizeof resp);
> +	INIT_UDATA(&udata, buf + sizeof(cmd),
> +		   (unsigned long) cmd.response + sizeof(resp),
> +		   in_len - sizeof(cmd) - sizeof(struct ib_uverbs_cmd_hdr),
> +		   out_len - sizeof(resp));
>
>  	ret = ib_rdmacg_try_charge(&cg_obj, ib_dev, RDMACG_RESOURCE_HCA_HANDLE);
>  	if (ret)
> @@ -313,9 +314,10 @@ ssize_t ib_uverbs_alloc_pd(struct ib_uverbs_file *file,
>  	if (copy_from_user(&cmd, buf, sizeof cmd))
>  		return -EFAULT;
>
> -	INIT_UDATA(&udata, buf + sizeof cmd,
> -		   (unsigned long) cmd.response + sizeof resp,
> -		   in_len - sizeof cmd, out_len - sizeof resp);
> +	INIT_UDATA(&udata, buf + sizeof(cmd),
> +		   (unsigned long) cmd.response + sizeof(resp),
> +                   in_len - sizeof(cmd) - sizeof(struct ib_uverbs_cmd_hdr),
> +                   out_len - sizeof(resp));
>
>  	uobj  = uobj_alloc(uobj_get_type(pd), file->ucontext);
>  	if (IS_ERR(uobj))
> @@ -482,9 +484,10 @@ ssize_t ib_uverbs_open_xrcd(struct ib_uverbs_file *file,
>  	if (copy_from_user(&cmd, buf, sizeof cmd))
>  		return -EFAULT;
>
> -	INIT_UDATA(&udata, buf + sizeof cmd,
> -		   (unsigned long) cmd.response + sizeof resp,
> -		   in_len - sizeof cmd, out_len - sizeof  resp);
> +	INIT_UDATA(&udata, buf + sizeof(cmd),
> +		   (unsigned long) cmd.response + sizeof(resp),
> +                   in_len - sizeof(cmd) - sizeof(struct ib_uverbs_cmd_hdr),
> +                   out_len - sizeof(resp));
>
>  	mutex_lock(&file->device->xrcd_tree_mutex);
>
> @@ -646,9 +649,10 @@ ssize_t ib_uverbs_reg_mr(struct ib_uverbs_file *file,
>  	if (copy_from_user(&cmd, buf, sizeof cmd))
>  		return -EFAULT;
>
> -	INIT_UDATA(&udata, buf + sizeof cmd,
> -		   (unsigned long) cmd.response + sizeof resp,
> -		   in_len - sizeof cmd, out_len - sizeof resp);
> +	INIT_UDATA(&udata, buf + sizeof(cmd),
> +		   (unsigned long) cmd.response + sizeof(resp),
> +                   in_len - sizeof(cmd) - sizeof(struct ib_uverbs_cmd_hdr),
> +                   out_len - sizeof(resp));
>
>  	if ((cmd.start & ~PAGE_MASK) != (cmd.hca_va & ~PAGE_MASK))
>  		return -EINVAL;
> @@ -740,7 +744,8 @@ ssize_t ib_uverbs_rereg_mr(struct ib_uverbs_file *file,
>
>  	INIT_UDATA(&udata, buf + sizeof(cmd),
>  		   (unsigned long) cmd.response + sizeof(resp),
> -		   in_len - sizeof(cmd), out_len - sizeof(resp));
> +                   in_len - sizeof(cmd) - sizeof(struct ib_uverbs_cmd_hdr),
> +                   out_len - sizeof(resp));
>
>  	if (cmd.flags & ~IB_MR_REREG_SUPPORTED || !cmd.flags)
>  		return -EINVAL;
> @@ -1080,7 +1085,8 @@ ssize_t ib_uverbs_create_cq(struct ib_uverbs_file *file,
>
>  	INIT_UDATA(&uhw, buf + sizeof(cmd),
>  		   (unsigned long)cmd.response + sizeof(resp),
> -		   in_len - sizeof(cmd), out_len - sizeof(resp));
> +		   in_len - sizeof(cmd) - sizeof(struct ib_uverbs_cmd_hdr),
> +		   out_len - sizeof(resp));
>
>  	memset(&cmd_ex, 0, sizeof(cmd_ex));
>  	cmd_ex.user_handle = cmd.user_handle;
> @@ -1161,9 +1167,10 @@ ssize_t ib_uverbs_resize_cq(struct ib_uverbs_file *file,
>  	if (copy_from_user(&cmd, buf, sizeof cmd))
>  		return -EFAULT;
>
> -	INIT_UDATA(&udata, buf + sizeof cmd,
> -		   (unsigned long) cmd.response + sizeof resp,
> -		   in_len - sizeof cmd, out_len - sizeof resp);
> +	INIT_UDATA(&udata, buf + sizeof(cmd),
> +		   (unsigned long) cmd.response + sizeof(resp),
> +		   in_len - sizeof(cmd) - sizeof(struct ib_uverbs_cmd_hdr),
> +		   out_len - sizeof(resp));
>
>  	cq = uobj_get_obj_read(cq, cmd.cq_handle, file->ucontext);
>  	if (!cq)
> @@ -1719,9 +1726,10 @@ ssize_t ib_uverbs_open_qp(struct ib_uverbs_file *file,
>  	if (copy_from_user(&cmd, buf, sizeof cmd))
>  		return -EFAULT;
>
> -	INIT_UDATA(&udata, buf + sizeof cmd,
> -		   (unsigned long) cmd.response + sizeof resp,
> -		   in_len - sizeof cmd, out_len - sizeof resp);
> +	INIT_UDATA(&udata, buf + sizeof(cmd),
> +		   (unsigned long) cmd.response + sizeof(resp),
> +		   in_len - sizeof(cmd) - sizeof(struct ib_uverbs_cmd_hdr),
> +		   out_len - sizeof(resp));
>
>  	obj  = (struct ib_uqp_object *)uobj_alloc(uobj_get_type(qp),
>  						  file->ucontext);
> @@ -2038,7 +2046,8 @@ ssize_t ib_uverbs_modify_qp(struct ib_uverbs_file *file,
>  		return -EOPNOTSUPP;
>
>  	INIT_UDATA(&udata, buf + sizeof(cmd.base), NULL,
> -		   in_len - sizeof(cmd.base), out_len);
> +		   in_len - sizeof(cmd.base) - sizeof(struct ib_uverbs_cmd_hdr),
> +		   out_len);
>
>  	ret = modify_qp(file, &cmd, &udata);
>  	if (ret)
> @@ -2543,7 +2552,8 @@ ssize_t ib_uverbs_create_ah(struct ib_uverbs_file *file,
>
>  	INIT_UDATA(&udata, buf + sizeof(cmd),
>  		   (unsigned long)cmd.response + sizeof(resp),
> -		   in_len - sizeof(cmd), out_len - sizeof(resp));
> +		   in_len - sizeof(cmd) - sizeof(struct ib_uverbs_cmd_hdr),
> +		   out_len - sizeof(resp));
>
>  	uobj  = uobj_alloc(uobj_get_type(ah), file->ucontext);
>  	if (IS_ERR(uobj))
> @@ -3609,10 +3619,10 @@ ssize_t ib_uverbs_create_srq(struct ib_uverbs_file *file,
>  	xcmd.max_sge	 = cmd.max_sge;
>  	xcmd.srq_limit	 = cmd.srq_limit;
>
> -	INIT_UDATA(&udata, buf + sizeof cmd,
> -		   (unsigned long) cmd.response + sizeof resp,
> -		   in_len - sizeof cmd - sizeof(struct ib_uverbs_cmd_hdr),
> -		   out_len - sizeof resp);
> +	INIT_UDATA(&udata, buf + sizeof(cmd),
> +		   (unsigned long) cmd.response + sizeof(resp),
> +		   in_len - sizeof(cmd) - sizeof(struct ib_uverbs_cmd_hdr),
> +		   out_len - sizeof(resp));

It is cleanup and should be placed in different patch, it is too
distracting to have it here.

>
>  	ret = __uverbs_create_xsrq(file, ib_dev, &xcmd, &udata);
>  	if (ret)
> @@ -3636,10 +3646,10 @@ ssize_t ib_uverbs_create_xsrq(struct ib_uverbs_file *file,
>  	if (copy_from_user(&cmd, buf, sizeof cmd))
>  		return -EFAULT;
>
> -	INIT_UDATA(&udata, buf + sizeof cmd,
> -		   (unsigned long) cmd.response + sizeof resp,
> -		   in_len - sizeof cmd - sizeof(struct ib_uverbs_cmd_hdr),
> -		   out_len - sizeof resp);
> +	INIT_UDATA(&udata, buf + sizeof(cmd),
> +		   (unsigned long) cmd.response + sizeof(resp),
> +		   in_len - sizeof(cmd) - sizeof(struct ib_uverbs_cmd_hdr),
> +		   out_len - sizeof(resp));
>
>  	ret = __uverbs_create_xsrq(file, ib_dev, &cmd, &udata);
>  	if (ret)
> diff --git a/drivers/infiniband/hw/mlx5/cq.c b/drivers/infiniband/hw/mlx5/cq.c
> index 94c049b..619e265 100644
> --- a/drivers/infiniband/hw/mlx5/cq.c
> +++ b/drivers/infiniband/hw/mlx5/cq.c
> @@ -751,10 +751,8 @@ static int create_cq_user(struct mlx5_ib_dev *dev, struct ib_udata *udata,
>  	void *cqc;
>  	int err;
>
> -	ucmdlen =
> -		(udata->inlen - sizeof(struct ib_uverbs_cmd_hdr) <
> -		 sizeof(ucmd)) ? (sizeof(ucmd) -
> -				  sizeof(ucmd.reserved)) : sizeof(ucmd);
> +	ucmdlen = udata->inlen < sizeof(ucmd) ?
> +		  (sizeof(ucmd) - sizeof(ucmd.reserved)) : sizeof(ucmd);
>
>  	if (ib_copy_from_udata(&ucmd, udata, ucmdlen))
>  		return -EFAULT;
> diff --git a/drivers/infiniband/hw/mlx5/main.c b/drivers/infiniband/hw/mlx5/main.c
> index 9ecc089..906baf0 100644
> --- a/drivers/infiniband/hw/mlx5/main.c
> +++ b/drivers/infiniband/hw/mlx5/main.c
> @@ -1211,7 +1211,6 @@ static struct ib_ucontext *mlx5_ib_alloc_ucontext(struct ib_device *ibdev,
>  	struct mlx5_bfreg_info *bfregi;
>  	int ver;
>  	int err;
> -	size_t reqlen;
>  	size_t min_req_v2 = offsetof(struct mlx5_ib_alloc_ucontext_req_v2,
>  				     max_cqe_version);
>  	bool lib_uar_4k;
> @@ -1219,18 +1218,14 @@ static struct ib_ucontext *mlx5_ib_alloc_ucontext(struct ib_device *ibdev,
>  	if (!dev->ib_active)
>  		return ERR_PTR(-EAGAIN);
>
> -	if (udata->inlen < sizeof(struct ib_uverbs_cmd_hdr))
> -		return ERR_PTR(-EINVAL);
> -
> -	reqlen = udata->inlen - sizeof(struct ib_uverbs_cmd_hdr);
> -	if (reqlen == sizeof(struct mlx5_ib_alloc_ucontext_req))
> +	if (udata->inlen == sizeof(struct mlx5_ib_alloc_ucontext_req))
>  		ver = 0;
> -	else if (reqlen >= min_req_v2)
> +	else if (udata->inlen >= min_req_v2)
>  		ver = 2;
>  	else
>  		return ERR_PTR(-EINVAL);
>
> -	err = ib_copy_from_udata(&req, udata, min(reqlen, sizeof(req)));
> +	err = ib_copy_from_udata(&req, udata, min(udata->inlen, sizeof(req)));
>  	if (err)
>  		return ERR_PTR(err);
>
> diff --git a/drivers/infiniband/hw/mthca/mthca_provider.c b/drivers/infiniband/hw/mthca/mthca_provider.c
> index c197cd9..1a222dc 100644
> --- a/drivers/infiniband/hw/mthca/mthca_provider.c
> +++ b/drivers/infiniband/hw/mthca/mthca_provider.c
> @@ -914,7 +914,7 @@ static struct ib_mr *mthca_reg_user_mr(struct ib_pd *pd, u64 start, u64 length,
>  	int err = 0;
>  	int write_mtt_size;
>
> -	if (udata->inlen - sizeof (struct ib_uverbs_cmd_hdr) < sizeof ucmd) {
> +	if (udata->inlen < sizeof ucmd) {
>  		if (!to_mucontext(pd->uobject->context)->reg_mr_warned) {
>  			mthca_warn(dev, "Process '%s' did not pass in MR attrs.\n",
>  				   current->comm);
> --
> 1.8.3.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

RE: [PATCH rdma v1 1/1] IB/core: Fix input len in multiple user verbs

[Amrani, Ram]

> > Most user verbs pass user data to the kernel with the inclusion of the
> > ib_uverbs_cmd_hdr structure. This is problematic because the vendor has
> > no ideas if the verb was called by a legacy verb or an extended verb.
> 
> Why vendor should know about it? It has midlayer (ib/core) between him
> and user to handle it.
> 

Knowing the inlen can be used to determine if the library is newer than
the kernel and what features it supports or not.

Ram


--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma v1 1/1] IB/core: Fix input len in multiple user verbs

[Leon Romanovsky]

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

On Wed, Jun 28, 2017 at 10:02:45AM +0000, Amrani, Ram wrote:
> > > Most user verbs pass user data to the kernel with the inclusion of the
> > > ib_uverbs_cmd_hdr structure. This is problematic because the vendor has
> > > no ideas if the verb was called by a legacy verb or an extended verb.
> >
> > Why vendor should know about it? It has midlayer (ib/core) between him
> > and user to handle it.
> >
>
> Knowing the inlen can be used to determine if the library is newer than
> the kernel and what features it supports or not.

It is not interesting case, because we are not breaking UAPI, only
extending. It ensures that kernel will fill as much as possible and will
always have success in it. After that it is library responsibility to
understand if everything was filled.

Thanks

>
> Ram
>
>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

RE: [PATCH rdma v1 1/1] IB/core: Fix input len in multiple user verbs

[Elior, Ariel]

> From: Leon Romanovsky [mailto:leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org]
> Sent: Wednesday, June 28, 2017 1:11 PM
> To: Amrani, Ram <Ram.Amrani-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
> Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org; linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Elior, Ariel
> <Ariel.Elior-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
> Subject: Re: [PATCH rdma v1 1/1] IB/core: Fix input len in multiple user verbs
> 
> On Wed, Jun 28, 2017 at 10:02:45AM +0000, Amrani, Ram wrote:
> > > > Most user verbs pass user data to the kernel with the inclusion of the
> > > > ib_uverbs_cmd_hdr structure. This is problematic because the vendor has
> > > > no ideas if the verb was called by a legacy verb or an extended verb.
> > >
> > > Why vendor should know about it? It has midlayer (ib/core) between him
> > > and user to handle it.
> > >
> >
> > Knowing the inlen can be used to determine if the library is newer than
> > the kernel and what features it supports or not.
> 
> It is not interesting case, because we are not breaking UAPI, only
> extending. It ensures that kernel will fill as much as possible and will
> always have success in it. After that it is library responsibility to
> understand if everything was filled.
> 
> Thanks

In our case, kernel driver needs to know whether library supports a certain
feature (doorbell overflow recovery). It doesn't care if it was an extended
verb or not, but it cares whether the lib is sufficiently new to have provided
the required information. If the library is sufficiently new (supports) then
kernel will register the library's doorbell address with the overflow recovery
mechanism, and perform recovery if required, otherwise it would not (as the
library is not supplying the necessary information for recovery to take place).
This is not something the library needs to know, but the kernel driver. Having
correct input len is required for successfully avoiding breaking the UAPI.

Ariel
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH rdma v1 0/1] IB/core: Fix input len in multiple user verbs

[Amrani, Ram]

> This patch follows [1] that fixed three verbs and extends that fix to
> the other verbs as well.
> 
> This fix is required for qedr to support existing and future libraries.
> 
> I was able to test the fix only partially due to its scope hence careful
> review is required, and testing, of course.
> 
> v0->v1:
>  * Updated vendor verbs to match the core changes.
> 
> 
> [1] https://www.spinics.net/lists/linux-rdma/msg33405.html
> 
> Ram Amrani (1):
>   IB/core: Fix input len in multiple user verbs
> 
>  drivers/infiniband/core/uverbs_cmd.c         | 70 ++++++++++++++++------------
>  drivers/infiniband/hw/mlx5/cq.c              |  6 +--
>  drivers/infiniband/hw/mlx5/main.c            | 11 ++---
>  drivers/infiniband/hw/mthca/mthca_provider.c |  2 +-
>  4 files changed, 46 insertions(+), 43 deletions(-)
> 
> --
> 1.8.3.1

Hi Doug,
Are you OK with applying the patch or do you expect ACKs for mlx5 and mthca?

Thanks,
Ram

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma v1 1/1] IB/core: Fix input len in multiple user verbs

[Doug Ledford]

On Wed, 2017-06-28 at 10:23 +0000, Elior, Ariel wrote:
> > From: Leon Romanovsky [mailto:leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org]
> > Sent: Wednesday, June 28, 2017 1:11 PM
> > To: Amrani, Ram <Ram.Amrani-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
> > Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org; linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Elior, Ariel
> > <Ariel.Elior-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
> > Subject: Re: [PATCH rdma v1 1/1] IB/core: Fix input len in multiple
> > user verbs
> > 
> > On Wed, Jun 28, 2017 at 10:02:45AM +0000, Amrani, Ram wrote:
> > > > > Most user verbs pass user data to the kernel with the
> > > > > inclusion of the
> > > > > ib_uverbs_cmd_hdr structure. This is problematic because the
> > > > > vendor has
> > > > > no ideas if the verb was called by a legacy verb or an
> > > > > extended verb.
> > > > 
> > > > Why vendor should know about it? It has midlayer (ib/core)
> > > > between him
> > > > and user to handle it.
> > > > 
> > > 
> > > Knowing the inlen can be used to determine if the library is
> > > newer than
> > > the kernel and what features it supports or not.
> > 
> > It is not interesting case, because we are not breaking UAPI, only
> > extending. It ensures that kernel will fill as much as possible and
> > will
> > always have success in it. After that it is library responsibility
> > to
> > understand if everything was filled.
> > 
> > Thanks
> 
> In our case, kernel driver needs to know whether library supports a
> certain
> feature (doorbell overflow recovery). It doesn't care if it was an
> extended
> verb or not, but it cares whether the lib is sufficiently new to have
> provided
> the required information. If the library is sufficiently new
> (supports) then
> kernel will register the library's doorbell address with the overflow
> recovery
> mechanism, and perform recovery if required, otherwise it would not
> (as the
> library is not supplying the necessary information for recovery to
> take place).
> This is not something the library needs to know, but the kernel
> driver. Having
> correct input len is required for successfully avoiding breaking the
> UAPI.

Leon, does this explanation resolve your objections?  Also, have you
checked the mlx5/mthca portions of this patch?

-- 
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
    GPG KeyID: B826A3330E572FDD
    Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma v1 0/1] IB/core: Fix input len in multiple user verbs

[Doug Ledford]

On Thu, 2017-07-06 at 12:24 +0000, Amrani, Ram wrote:
> > This patch follows [1] that fixed three verbs and extends that fix
> > to
> > the other verbs as well.
> > 
> > This fix is required for qedr to support existing and future
> > libraries.
> > 
> > I was able to test the fix only partially due to its scope hence
> > careful
> > review is required, and testing, of course.
> > 
> > v0->v1:
> >  * Updated vendor verbs to match the core changes.
> > 
> > 
> > [1] https://www.spinics.net/lists/linux-rdma/msg33405.html
> > 
> > Ram Amrani (1):
> >   IB/core: Fix input len in multiple user verbs
> > 
> >  drivers/infiniband/core/uverbs_cmd.c         | 70
> > ++++++++++++++++------------
> >  drivers/infiniband/hw/mlx5/cq.c              |  6 +--
> >  drivers/infiniband/hw/mlx5/main.c            | 11 ++---
> >  drivers/infiniband/hw/mthca/mthca_provider.c |  2 +-
> >  4 files changed, 46 insertions(+), 43 deletions(-)
> > 
> > --
> > 1.8.3.1
> 
> Hi Doug,
> Are you OK with applying the patch or do you expect ACKs for mlx5 and
> mthca?

I do want to make sure that Leon has looked over the mlx5 and mthca
sections, yes.  I could run regression tests, but I'd rather him say
that the changes are OK as opposed to relying on test results.

-- 
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
    GPG KeyID: B826A3330E572FDD
    Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma v1 1/1] IB/core: Fix input len in multiple user verbs

[Leon Romanovsky]

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

On Fri, Aug 18, 2017 at 11:00:04AM -0400, Doug Ledford wrote:
> On Wed, 2017-06-28 at 10:23 +0000, Elior, Ariel wrote:
> > > From: Leon Romanovsky [mailto:leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org]
> > > Sent: Wednesday, June 28, 2017 1:11 PM
> > > To: Amrani, Ram <Ram.Amrani-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
> > > Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org; linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Elior, Ariel
> > > <Ariel.Elior-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
> > > Subject: Re: [PATCH rdma v1 1/1] IB/core: Fix input len in multiple
> > > user verbs
> > >
> > > On Wed, Jun 28, 2017 at 10:02:45AM +0000, Amrani, Ram wrote:
> > > > > > Most user verbs pass user data to the kernel with the
> > > > > > inclusion of the
> > > > > > ib_uverbs_cmd_hdr structure. This is problematic because the
> > > > > > vendor has
> > > > > > no ideas if the verb was called by a legacy verb or an
> > > > > > extended verb.
> > > > >
> > > > > Why vendor should know about it? It has midlayer (ib/core)
> > > > > between him
> > > > > and user to handle it.
> > > > >
> > > >
> > > > Knowing the inlen can be used to determine if the library is
> > > > newer than
> > > > the kernel and what features it supports or not.
> > >
> > > It is not interesting case, because we are not breaking UAPI, only
> > > extending. It ensures that kernel will fill as much as possible and
> > > will
> > > always have success in it. After that it is library responsibility
> > > to
> > > understand if everything was filled.
> > >
> > > Thanks
> >
> > In our case, kernel driver needs to know whether library supports a
> > certain
> > feature (doorbell overflow recovery). It doesn't care if it was an
> > extended
> > verb or not, but it cares whether the lib is sufficiently new to have
> > provided
> > the required information. If the library is sufficiently new
> > (supports) then
> > kernel will register the library's doorbell address with the overflow
> > recovery
> > mechanism, and perform recovery if required, otherwise it would not
> > (as the
> > library is not supplying the necessary information for recovery to
> > take place).
> > This is not something the library needs to know, but the kernel
> > driver. Having
> > correct input len is required for successfully avoiding breaking the
> > UAPI.
>
> Leon, does this explanation resolve your objections?  Also, have you
> checked the mlx5/mthca portions of this patch?

Yes, let's me recheck/rerun the patch and post results.
If for any reasons you don't hear from me till Wednesday, take it.

Thanks

>
> --
> Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>     GPG KeyID: B826A3330E572FDD
>     Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD
>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH rdma v1 1/1] IB/core: Fix input len in multiple user verbs

[Doug Ledford]

On Fri, 2017-08-18 at 18:58 +0300, Leon Romanovsky wrote:
> On Fri, Aug 18, 2017 at 11:00:04AM -0400, Doug Ledford wrote:
> > On Wed, 2017-06-28 at 10:23 +0000, Elior, Ariel wrote:
> > > > From: Leon Romanovsky [mailto:leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org]
> > > > Sent: Wednesday, June 28, 2017 1:11 PM
> > > > To: Amrani, Ram <Ram.Amrani-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
> > > > Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org; linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Elior,
> > > > Ariel
> > > > <Ariel.Elior-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
> > > > Subject: Re: [PATCH rdma v1 1/1] IB/core: Fix input len in
> > > > multiple
> > > > user verbs
> > > > 
> > > > On Wed, Jun 28, 2017 at 10:02:45AM +0000, Amrani, Ram wrote:
> > > > > > > Most user verbs pass user data to the kernel with the
> > > > > > > inclusion of the
> > > > > > > ib_uverbs_cmd_hdr structure. This is problematic because
> > > > > > > the
> > > > > > > vendor has
> > > > > > > no ideas if the verb was called by a legacy verb or an
> > > > > > > extended verb.
> > > > > > 
> > > > > > Why vendor should know about it? It has midlayer (ib/core)
> > > > > > between him
> > > > > > and user to handle it.
> > > > > > 
> > > > > 
> > > > > Knowing the inlen can be used to determine if the library is
> > > > > newer than
> > > > > the kernel and what features it supports or not.
> > > > 
> > > > It is not interesting case, because we are not breaking UAPI,
> > > > only
> > > > extending. It ensures that kernel will fill as much as possible
> > > > and
> > > > will
> > > > always have success in it. After that it is library
> > > > responsibility
> > > > to
> > > > understand if everything was filled.
> > > > 
> > > > Thanks
> > > 
> > > In our case, kernel driver needs to know whether library supports
> > > a
> > > certain
> > > feature (doorbell overflow recovery). It doesn't care if it was
> > > an
> > > extended
> > > verb or not, but it cares whether the lib is sufficiently new to
> > > have
> > > provided
> > > the required information. If the library is sufficiently new
> > > (supports) then
> > > kernel will register the library's doorbell address with the
> > > overflow
> > > recovery
> > > mechanism, and perform recovery if required, otherwise it would
> > > not
> > > (as the
> > > library is not supplying the necessary information for recovery
> > > to
> > > take place).
> > > This is not something the library needs to know, but the kernel
> > > driver. Having
> > > correct input len is required for successfully avoiding breaking
> > > the
> > > UAPI.
> > 
> > Leon, does this explanation resolve your objections?  Also, have
> > you
> > checked the mlx5/mthca portions of this patch?
> 
> Yes, let's me recheck/rerun the patch and post results.
> If for any reasons you don't hear from me till Wednesday, take it.
> 
> Thanks

Works for me, thanks.

-- 
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
    GPG KeyID: B826A3330E572FDD
    Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH rdma v1 1/1] IB/core: Fix input len in multiple user verbs

[Leon Romanovsky]

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

On Fri, Aug 18, 2017 at 12:07:45PM -0400, Doug Ledford wrote:
> On Fri, 2017-08-18 at 18:58 +0300, Leon Romanovsky wrote:
> > On Fri, Aug 18, 2017 at 11:00:04AM -0400, Doug Ledford wrote:
> > > On Wed, 2017-06-28 at 10:23 +0000, Elior, Ariel wrote:
> > > > > From: Leon Romanovsky [mailto:leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org]
> > > > > Sent: Wednesday, June 28, 2017 1:11 PM
> > > > > To: Amrani, Ram <Ram.Amrani-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
> > > > > Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org; linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Elior,
> > > > > Ariel
> > > > > <Ariel.Elior-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
> > > > > Subject: Re: [PATCH rdma v1 1/1] IB/core: Fix input len in
> > > > > multiple
> > > > > user verbs
> > > > >
> > > > > On Wed, Jun 28, 2017 at 10:02:45AM +0000, Amrani, Ram wrote:
> > > > > > > > Most user verbs pass user data to the kernel with the
> > > > > > > > inclusion of the
> > > > > > > > ib_uverbs_cmd_hdr structure. This is problematic because
> > > > > > > > the
> > > > > > > > vendor has
> > > > > > > > no ideas if the verb was called by a legacy verb or an
> > > > > > > > extended verb.
> > > > > > >
> > > > > > > Why vendor should know about it? It has midlayer (ib/core)
> > > > > > > between him
> > > > > > > and user to handle it.
> > > > > > >
> > > > > >
> > > > > > Knowing the inlen can be used to determine if the library is
> > > > > > newer than
> > > > > > the kernel and what features it supports or not.
> > > > >
> > > > > It is not interesting case, because we are not breaking UAPI,
> > > > > only
> > > > > extending. It ensures that kernel will fill as much as possible
> > > > > and
> > > > > will
> > > > > always have success in it. After that it is library
> > > > > responsibility
> > > > > to
> > > > > understand if everything was filled.
> > > > >
> > > > > Thanks
> > > >
> > > > In our case, kernel driver needs to know whether library supports
> > > > a
> > > > certain
> > > > feature (doorbell overflow recovery). It doesn't care if it was
> > > > an
> > > > extended
> > > > verb or not, but it cares whether the lib is sufficiently new to
> > > > have
> > > > provided
> > > > the required information. If the library is sufficiently new
> > > > (supports) then
> > > > kernel will register the library's doorbell address with the
> > > > overflow
> > > > recovery
> > > > mechanism, and perform recovery if required, otherwise it would
> > > > not
> > > > (as the
> > > > library is not supplying the necessary information for recovery
> > > > to
> > > > take place).
> > > > This is not something the library needs to know, but the kernel
> > > > driver. Having
> > > > correct input len is required for successfully avoiding breaking
> > > > the
> > > > UAPI.
> > >
> > > Leon, does this explanation resolve your objections?  Also, have
> > > you
> > > checked the mlx5/mthca portions of this patch?
> >
> > Yes, let's me recheck/rerun the patch and post results.
> > If for any reasons you don't hear from me till Wednesday, take it.
> >
> > Thanks
>
> Works for me, thanks.

The series worked for us and we didn't encounter unknown failures.

Thanks

>
> --
> Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>     GPG KeyID: B826A3330E572FDD
>     Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD
>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH rdma v1 1/1] IB/core: Fix input len in multiple user verbs

[Doug Ledford]

On Tue, 2017-08-22 at 15:40 +0300, Leon Romanovsky wrote:
> > > Yes, let's me recheck/rerun the patch and post results.
> > > If for any reasons you don't hear from me till Wednesday, take
> it.
> > >
> > > Thanks
> >
> > Works for me, thanks.
> 
> The series worked for us and we didn't encounter unknown failures.
> 
> Thanks

Patch applied, thanks.

-- 
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
    GPG KeyID: B826A3330E572FDD
    Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html