[PATCH] RDMA/mlx5: Validate the DEVX command length before setting umem valid bits

[PATCH] RDMA/mlx5: Validate the DEVX command length before setting umem valid bits

[Hungyu Lin]

devx_set_umem_valid() sets the umem valid bits in the command supplied
by user space, at offsets that depend on the opcode. The DEVX object
create and modify methods only require the command to be as long as
general_obj_in_cmd_hdr, which is 16 bytes, so when the command is
shorter than the layout of its opcode the bits are written past the end
of the command buffer. For MLX5_CMD_OP_CREATE_QP the modified words are
232 and 268 bytes into the command.

The command buffer is allocated with uverbs_alloc(), so the write lands
either past the internal buffer of the bundle, which normally lives on
the stack of ib_uverbs_cmd_verbs(), or past a separate allocation.

Fail with -EINVAL when the command does not cover the layout that is
modified, as devx_handle_mkey_create() already does for
MLX5_CMD_OP_CREATE_MKEY.

Fixes: ba1a057da2f1 ("IB/mlx5: Set valid umem bit on DEVX")
Cc: stable@vger.kernel.org
Signed-off-by: Hungyu Lin <dennylin0707@gmail.com>
---
 drivers/infiniband/hw/mlx5/devx.c | 44 ++++++++++++++++++++++++++++++++-------
 1 file changed, 37 insertions(+), 7 deletions(-)

diff --git a/drivers/infiniband/hw/mlx5/devx.c b/drivers/infiniband/hw/mlx5/devx.c
index fc6f793dcc65..5eb104994b4a 100644
--- a/drivers/infiniband/hw/mlx5/devx.c
+++ b/drivers/infiniband/hw/mlx5/devx.c
@@ -730,7 +730,7 @@ static bool devx_is_valid_obj_id(struct uverbs_attr_bundle *attrs,
 	}
 }
 
-static void devx_set_umem_valid(const void *in)
+static int devx_set_umem_valid(const void *in, int in_len)
 {
 	u16 opcode = MLX5_GET(general_obj_in_cmd_hdr, in, opcode);
 
@@ -742,6 +742,9 @@ static void devx_set_umem_valid(const void *in)
 	{
 		void *cqc;
 
+		if (in_len < MLX5_ST_SZ_BYTES(create_cq_in))
+			return -EINVAL;
+
 		MLX5_SET(create_cq_in, in, cq_umem_valid, 1);
 		cqc = MLX5_ADDR_OF(create_cq_in, in, cq_context);
 		MLX5_SET(cqc, cqc, dbr_umem_valid, 1);
@@ -751,6 +754,9 @@ static void devx_set_umem_valid(const void *in)
 	{
 		void *qpc;
 
+		if (in_len < MLX5_ST_SZ_BYTES(create_qp_in))
+			return -EINVAL;
+
 		qpc = MLX5_ADDR_OF(create_qp_in, in, qpc);
 		MLX5_SET(qpc, qpc, dbr_umem_valid, 1);
 		MLX5_SET(create_qp_in, in, wq_umem_valid, 1);
@@ -761,6 +767,9 @@ static void devx_set_umem_valid(const void *in)
 	{
 		void *rqc, *wq;
 
+		if (in_len < MLX5_ST_SZ_BYTES(create_rq_in))
+			return -EINVAL;
+
 		rqc = MLX5_ADDR_OF(create_rq_in, in, ctx);
 		wq  = MLX5_ADDR_OF(rqc, rqc, wq);
 		MLX5_SET(wq, wq, dbr_umem_valid, 1);
@@ -772,6 +781,9 @@ static void devx_set_umem_valid(const void *in)
 	{
 		void *sqc, *wq;
 
+		if (in_len < MLX5_ST_SZ_BYTES(create_sq_in))
+			return -EINVAL;
+
 		sqc = MLX5_ADDR_OF(create_sq_in, in, ctx);
 		wq = MLX5_ADDR_OF(sqc, sqc, wq);
 		MLX5_SET(wq, wq, dbr_umem_valid, 1);
@@ -780,6 +792,9 @@ static void devx_set_umem_valid(const void *in)
 	}
 
 	case MLX5_CMD_OP_MODIFY_CQ:
+		if (in_len < MLX5_ST_SZ_BYTES(modify_cq_in))
+			return -EINVAL;
+
 		MLX5_SET(modify_cq_in, in, cq_umem_valid, 1);
 		break;
 
@@ -787,6 +802,9 @@ static void devx_set_umem_valid(const void *in)
 	{
 		void *rmpc, *wq;
 
+		if (in_len < MLX5_ST_SZ_BYTES(create_rmp_in))
+			return -EINVAL;
+
 		rmpc = MLX5_ADDR_OF(create_rmp_in, in, ctx);
 		wq = MLX5_ADDR_OF(rmpc, rmpc, wq);
 		MLX5_SET(wq, wq, dbr_umem_valid, 1);
@@ -798,6 +816,9 @@ static void devx_set_umem_valid(const void *in)
 	{
 		void *xrqc, *wq;
 
+		if (in_len < MLX5_ST_SZ_BYTES(create_xrq_in))
+			return -EINVAL;
+
 		xrqc = MLX5_ADDR_OF(create_xrq_in, in, xrq_context);
 		wq = MLX5_ADDR_OF(xrqc, xrqc, wq);
 		MLX5_SET(wq, wq, dbr_umem_valid, 1);
@@ -809,6 +830,9 @@ static void devx_set_umem_valid(const void *in)
 	{
 		void *xrc_srqc;
 
+		if (in_len < MLX5_ST_SZ_BYTES(create_xrc_srq_in))
+			return -EINVAL;
+
 		MLX5_SET(create_xrc_srq_in, in, xrc_srq_umem_valid, 1);
 		xrc_srqc = MLX5_ADDR_OF(create_xrc_srq_in, in,
 					xrc_srq_context_entry);
@@ -817,8 +841,10 @@ static void devx_set_umem_valid(const void *in)
 	}
 
 	default:
-		return;
+		break;
 	}
+
+	return 0;
 }
 
 static bool devx_is_obj_create_cmd(const void *in, u16 *opcode)
@@ -1567,7 +1593,9 @@ static int UVERBS_HANDLER(MLX5_IB_METHOD_DEVX_OBJ_CREATE)(
 		if (err)
 			goto obj_free;
 	} else {
-		devx_set_umem_valid(cmd_in);
+		err = devx_set_umem_valid(cmd_in, cmd_in_len);
+		if (err)
+			goto obj_free;
 	}
 
 	if (opcode == MLX5_CMD_OP_CREATE_DCT) {
@@ -1646,6 +1674,8 @@ static int UVERBS_HANDLER(MLX5_IB_METHOD_DEVX_OBJ_MODIFY)(
 	struct uverbs_attr_bundle *attrs)
 {
 	void *cmd_in = uverbs_attr_get_alloced_ptr(attrs, MLX5_IB_ATTR_DEVX_OBJ_MODIFY_CMD_IN);
+	int cmd_in_len = uverbs_attr_get_len(attrs,
+					MLX5_IB_ATTR_DEVX_OBJ_MODIFY_CMD_IN);
 	int cmd_out_len = uverbs_attr_get_len(attrs,
 					MLX5_IB_ATTR_DEVX_OBJ_MODIFY_CMD_OUT);
 	struct ib_uobject *uobj = uverbs_attr_get_uobject(attrs,
@@ -1675,11 +1705,11 @@ static int UVERBS_HANDLER(MLX5_IB_METHOD_DEVX_OBJ_MODIFY)(
 		return PTR_ERR(cmd_out);
 
 	MLX5_SET(general_obj_in_cmd_hdr, cmd_in, uid, uid);
-	devx_set_umem_valid(cmd_in);
+	err = devx_set_umem_valid(cmd_in, cmd_in_len);
+	if (err)
+		return err;
 
-	err = mlx5_cmd_do(mdev->mdev, cmd_in,
-			  uverbs_attr_get_len(attrs, MLX5_IB_ATTR_DEVX_OBJ_MODIFY_CMD_IN),
-			  cmd_out, cmd_out_len);
+	err = mlx5_cmd_do(mdev->mdev, cmd_in, cmd_in_len, cmd_out, cmd_out_len);
 	if (err && err != -EREMOTEIO)
 		return err;
 
-- 
2.43.0

Re: [PATCH] RDMA/mlx5: Validate the DEVX command length before setting umem valid bits

[sashiko-bot]

> devx_set_umem_valid() sets the umem valid bits in the command supplied
> by user space, at offsets that depend on the opcode. The DEVX object
> create and modify methods only require the command to be as long as
> general_obj_in_cmd_hdr, which is 16 bytes, so when the command is
> shorter than the layout of its opcode the bits are written past the end
> of the command buffer. For MLX5_CMD_OP_CREATE_QP the modified words are
> 232 and 268 bytes into the command.
> 
> The command buffer is allocated with uverbs_alloc(), so the write lands
> either past the internal buffer of the bundle, which normally lives on
> the stack of ib_uverbs_cmd_verbs(), or past a separate allocation.
> 
> Fail with -EINVAL when the command does not cover the layout that is
> modified, as devx_handle_mkey_create() already does for
> MLX5_CMD_OP_CREATE_MKEY.
> 
> Fixes: ba1a057da2f1 ("IB/mlx5: Set valid umem bit on DEVX")
> Cc: stable@vger.kernel.org
> Signed-off-by: Hungyu Lin <dennylin0707@gmail.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261115164658.88420-1-dennylin0707@gmail.com?part=1