[PATCH rdma-rc v1] RDMA/core: Sanitize WQ state received from the userspace

[PATCH rdma-rc v1] RDMA/core: Sanitize WQ state received from the userspace

[Leon Romanovsky]

From: Leon Romanovsky <leonro@nvidia.com>

The mlx4 and mlx5 implemented differently the WQ input checks.
Instead of duplicating mlx4 logic in the mlx5, let's prepare
the input in the central place.

Fixes: f213c0527210 ("IB/uverbs: Add WQ support")
Reported-by: Jiapeng Chong <jiapeng.chong@linux.alibaba.com>
Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
Changelog:
v1:
 * Removed IB_WQS_RESET state checks because it is zero and wq states
   declared as u32, so can't be less than IB_WQS_RESET.
v0: https://lore.kernel.org/lkml/932f87b48c07278730c3c760b3a707d6a984b524.1621332736.git.leonro@nvidia.com
---
 drivers/infiniband/core/uverbs_cmd.c | 21 +++++++++++++++++++--
 drivers/infiniband/hw/mlx4/qp.c      |  9 ++-------
 drivers/infiniband/hw/mlx5/qp.c      |  6 ++----
 3 files changed, 23 insertions(+), 13 deletions(-)

diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
index 4f890bff80f8..c6f53d894411 100644
--- a/drivers/infiniband/core/uverbs_cmd.c
+++ b/drivers/infiniband/core/uverbs_cmd.c
@@ -3084,12 +3084,29 @@ static int ib_uverbs_ex_modify_wq(struct uverbs_attr_bundle *attrs)
 	if (!wq)
 		return -EINVAL;
 
-	wq_attr.curr_wq_state = cmd.curr_wq_state;
-	wq_attr.wq_state = cmd.wq_state;
 	if (cmd.attr_mask & IB_WQ_FLAGS) {
 		wq_attr.flags = cmd.flags;
 		wq_attr.flags_mask = cmd.flags_mask;
 	}
+
+	if (cmd.attr_mask & IB_WQ_CUR_STATE) {
+		if (cmd.curr_wq_state > IB_WQS_ERR)
+			return -EINVAL;
+
+		wq_attr.curr_wq_state = cmd.curr_wq_state;
+	} else {
+		wq_attr.curr_wq_state = wq->state;
+	}
+
+	if (cmd.attr_mask & IB_WQ_STATE) {
+		if (cmd.wq_state > IB_WQS_ERR)
+			return -EINVAL;
+
+		wq_attr.wq_state = cmd.wq_state;
+	} else {
+		wq_attr.wq_state = wq_attr.curr_wq_state;
+	}
+
 	ret = wq->device->ops.modify_wq(wq, &wq_attr, cmd.attr_mask,
 					&attrs->driver_udata);
 	rdma_lookup_put_uobject(&wq->uobject->uevent.uobject,
diff --git a/drivers/infiniband/hw/mlx4/qp.c b/drivers/infiniband/hw/mlx4/qp.c
index 92ddbcc00eb2..2ae22bf50016 100644
--- a/drivers/infiniband/hw/mlx4/qp.c
+++ b/drivers/infiniband/hw/mlx4/qp.c
@@ -4251,13 +4251,8 @@ int mlx4_ib_modify_wq(struct ib_wq *ibwq, struct ib_wq_attr *wq_attr,
 	if (wq_attr_mask & IB_WQ_FLAGS)
 		return -EOPNOTSUPP;
 
-	cur_state = wq_attr_mask & IB_WQ_CUR_STATE ? wq_attr->curr_wq_state :
-						     ibwq->state;
-	new_state = wq_attr_mask & IB_WQ_STATE ? wq_attr->wq_state : cur_state;
-
-	if (cur_state  < IB_WQS_RESET || cur_state  > IB_WQS_ERR ||
-	    new_state < IB_WQS_RESET || new_state > IB_WQS_ERR)
-		return -EINVAL;
+	cur_state = wq_attr->curr_wq_state;
+	new_state = wq_attr->wq_state;
 
 	if ((new_state == IB_WQS_RDY) && (cur_state == IB_WQS_ERR))
 		return -EINVAL;
diff --git a/drivers/infiniband/hw/mlx5/qp.c b/drivers/infiniband/hw/mlx5/qp.c
index d984b451c379..becd250388af 100644
--- a/drivers/infiniband/hw/mlx5/qp.c
+++ b/drivers/infiniband/hw/mlx5/qp.c
@@ -5483,10 +5483,8 @@ int mlx5_ib_modify_wq(struct ib_wq *wq, struct ib_wq_attr *wq_attr,
 
 	rqc = MLX5_ADDR_OF(modify_rq_in, in, ctx);
 
-	curr_wq_state = (wq_attr_mask & IB_WQ_CUR_STATE) ?
-		wq_attr->curr_wq_state : wq->state;
-	wq_state = (wq_attr_mask & IB_WQ_STATE) ?
-		wq_attr->wq_state : curr_wq_state;
+	curr_wq_state = wq_attr->curr_wq_state;
+	wq_state = wq_attr->wq_state;
 	if (curr_wq_state == IB_WQS_ERR)
 		curr_wq_state = MLX5_RQC_STATE_ERR;
 	if (wq_state == IB_WQS_ERR)
-- 
2.31.1

RE: [PATCH rdma-rc v1] RDMA/core: Sanitize WQ state received from the userspace

[Saleem, Shiraz]

> Subject: [PATCH rdma-rc v1] RDMA/core: Sanitize WQ state received from the
> userspace
> 
> From: Leon Romanovsky <leonro@nvidia.com>
> 
> The mlx4 and mlx5 implemented differently the WQ input checks.
> Instead of duplicating mlx4 logic in the mlx5, let's prepare the input in the central
> place.

Maybe some more verbiage about what the bug was in mlx5 that prompted this patch would
be good since this an -rc fix.

Shiraz

Re: [PATCH rdma-rc v1] RDMA/core: Sanitize WQ state received from the userspace

[Leon Romanovsky]

On Tue, May 18, 2021 at 10:07:38PM +0000, Saleem, Shiraz wrote:
> > Subject: [PATCH rdma-rc v1] RDMA/core: Sanitize WQ state received from the
> > userspace
> > 
> > From: Leon Romanovsky <leonro@nvidia.com>
> > 
> > The mlx4 and mlx5 implemented differently the WQ input checks.
> > Instead of duplicating mlx4 logic in the mlx5, let's prepare the input in the central
> > place.
> 
> Maybe some more verbiage about what the bug was in mlx5 that prompted this patch would
> be good since this an -rc fix.

It can go to -next too, the bug is that we didn't check for validity of
state input in mlx5. It is not real bug because our FW checked that, but
still worth to fix.

I'll add this.

Thanks

> 
> Shiraz