[PATCH] RDMA/mlx5: Fix UAF after DEVX DCT create failure

[PATCH] RDMA/mlx5: Fix UAF after DEVX DCT create failure

[Jaeyeong Lee]

The DEVX object create handler destroys and frees a hardware object itself
when copying the command output or setting up an indirect mkey fails.

For DCT objects, mlx5_core_destroy_dct() can fail while leaving the DCT in
qp_table.dct_xa. The error is ignored and the enclosing devx_obj is freed.
A later DCT_DRAINED event then loads the stale xarray entry and calls
complete() on freed memory.

Mark the uobject as holding a valid hardware object immediately after the
DEVX object is initialized. Let the uverbs abort path invoke
devx_obj_cleanup() for subsequent failures. If hardware destruction fails,
rdma_alloc_abort_uobject() commits the uobject instead of freeing it. This
keeps the object alive for cleanup at file close.

A KASAN QEMU reproducer that forces DRAIN_DCT to fail and delivers a
DCT_DRAINED event reports:

  BUG: KASAN: slab-use-after-free in _raw_spin_lock_irqsave
  complete
  rsc_event_notifier

The issue was found with a static analysis tool. OpenAI Codex assisted with
reproducer validation and patch preparation.

Fixes: c5ae1954c47d3 ("IB/mlx5: Use mlx5 core to create/destroy a DEVX DCT")
Cc: stable@vger.kernel.org
Signed-off-by: Jaeyeong Lee <lee@jaeyeong.cc>
---
 drivers/infiniband/hw/mlx5/devx.c | 14 +++-----------
 1 file changed, 3 insertions(+), 11 deletions(-)

diff --git a/drivers/infiniband/hw/mlx5/devx.c b/drivers/infiniband/hw/mlx5/devx.c
index fc6f793dcc658..037c2b6ff9714 100644
--- a/drivers/infiniband/hw/mlx5/devx.c
+++ b/drivers/infiniband/hw/mlx5/devx.c
@@ -1535,7 +1535,6 @@ static int UVERBS_HANDLER(MLX5_IB_METHOD_DEVX_OBJ_CREATE)(
 	struct mlx5_ib_ucontext *c = rdma_udata_to_drv_context(
 		&attrs->driver_udata, struct mlx5_ib_ucontext, ibucontext);
 	struct mlx5_ib_dev *dev = to_mdev(c->ibucontext.device);
-	u32 out[MLX5_ST_SZ_DW(general_obj_out_cmd_hdr)];
 	struct devx_obj *obj;
 	u16 obj_type = 0;
 	int err, err2 = 0;
@@ -1613,10 +1612,11 @@ static int UVERBS_HANDLER(MLX5_IB_METHOD_DEVX_OBJ_CREATE)(
 	devx_obj_build_destroy_cmd(cmd_in, cmd_out, obj->dinbox, &obj->dinlen,
 				   &obj_id);
 	WARN_ON(obj->dinlen > MLX5_MAX_DESTROY_INBOX_SIZE_DW * sizeof(u32));
+	uverbs_finalize_uobj_create(attrs, MLX5_IB_ATTR_DEVX_OBJ_CREATE_HANDLE);
 
 	err = uverbs_copy_to(attrs, MLX5_IB_ATTR_DEVX_OBJ_CREATE_CMD_OUT, cmd_out, cmd_out_len);
 	if (err)
-		goto obj_destroy;
+		return err;
 
 	if (opcode == MLX5_CMD_OP_CREATE_GENERAL_OBJECT)
 		obj_type = MLX5_GET(general_obj_in_cmd_hdr, cmd_in, obj_type);
@@ -1625,18 +1625,10 @@ static int UVERBS_HANDLER(MLX5_IB_METHOD_DEVX_OBJ_CREATE)(
 	if (obj->flags & DEVX_OBJ_FLAGS_INDIRECT_MKEY) {
 		err = devx_handle_mkey_indirect(obj, dev, cmd_in, cmd_out);
 		if (err)
-			goto obj_destroy;
+			return err;
 	}
 	return 0;
 
-obj_destroy:
-	if (obj->flags & DEVX_OBJ_FLAGS_DCT)
-		mlx5_core_destroy_dct(obj->ib_dev, &obj->core_dct);
-	else if (obj->flags & DEVX_OBJ_FLAGS_CQ)
-		mlx5_core_destroy_cq(obj->ib_dev->mdev, &obj->core_cq);
-	else
-		mlx5_cmd_exec(obj->ib_dev->mdev, obj->dinbox, obj->dinlen, out,
-			      sizeof(out));
 obj_free:
 	kfree(obj);
 	return err2 ?: err;

Re: [PATCH] RDMA/mlx5: Fix UAF after DEVX DCT create failure

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [Medium] RDMA/mlx5: uninitialized or uninserted mkey cleanup in
  devx_obj_cleanup()
- [Medium] RDMA/mlx5: uninitialized obj->obj_id on abort-commit in
  devx_obj_create
--

commit d3fab8d64df61af1d9b384a2ecf8764391f6c971
Author: Jaeyeong Lee <lee@jaeyeong.cc>

RDMA/mlx5: Fix UAF after DEVX DCT create failure

This commit calls uverbs_finalize_uobj_create() immediately after building
the DEVX destroy command and removes the local obj_destroy error path so
that uverbs abort cleanup handles subsequent failures.

> diff --git a/drivers/infiniband/hw/mlx5/devx.c b/drivers/infiniband/hw/mlx5/devx.c
> index fc6f793dcc658..037c2b6ff9714 100644
> --- a/drivers/infiniband/hw/mlx5/devx.c
> +++ b/drivers/infiniband/hw/mlx5/devx.c
[ ... ]
> @@ -1613,10 +1612,11 @@ static int UVERBS_HANDLER(MLX5_IB_METHOD_DEVX_OBJ_CREATE)(
>  	devx_obj_build_destroy_cmd(cmd_in, cmd_out, obj->dinbox, &obj->dinlen,
>  				   &obj_id);
>  	WARN_ON(obj->dinlen > MLX5_MAX_DESTROY_INBOX_SIZE_DW * sizeof(u32));
> +	uverbs_finalize_uobj_create(attrs, MLX5_IB_ATTR_DEVX_OBJ_CREATE_HANDLE);
>  
>  	err = uverbs_copy_to(attrs, MLX5_IB_ATTR_DEVX_OBJ_CREATE_CMD_OUT, cmd_out, cmd_out_len);
>  	if (err)
> -		goto obj_destroy;
> +		return err;
>  
>  	if (opcode == MLX5_CMD_OP_CREATE_GENERAL_OBJECT)
>  		obj_type = MLX5_GET(general_obj_in_cmd_hdr, cmd_in, obj_type);
> @@ -1625,18 +1625,10 @@ static int UVERBS_HANDLER(MLX5_IB_METHOD_DEVX_OBJ_CREATE)(

[Severity: Medium]
Can returning on a uverbs_copy_to() failure leave obj->obj_id uninitialized
when rdma_alloc_abort_uobject() commits uobj?

If uverbs_copy_to() fails and hardware destruction in devx_obj_cleanup()
also fails, rdma_alloc_abort_uobject() commits uobj with obj->obj_id still
zero.  At file close, mlx5_ib_ufile_hw_cleanup() then reads a zero opcode
from get_dec_obj_type() and misses MLX5_OBJ_TYPE_QP objects during async
teardown, while MLX5_IB_METHOD_DEVX_SUBSCRIBE_EVENT decodes both the object
type and ID as 0 and inserts event subscriptions into event_xa under the
wrong key.  Initializing obj->obj_id before uverbs_finalize_uobj_create()
avoids this.

>  	if (obj->flags & DEVX_OBJ_FLAGS_INDIRECT_MKEY) {
>  		err = devx_handle_mkey_indirect(obj, dev, cmd_in, cmd_out);
>  		if (err)
> -			goto obj_destroy;
> +			return err;
>  	}

[Severity: Medium]
Does calling uverbs_finalize_uobj_create() before devx_handle_mkey_indirect()
cause devx_obj_cleanup() to clean up an uninitialized or uninserted
obj->mkey?

devx_handle_mkey_create() sets DEVX_OBJ_FLAGS_INDIRECT_MKEY before the
command executes, whereas obj->mkey is only initialized and stored in
dev->odp_mkeys by devx_handle_mkey_indirect():

drivers/infiniband/hw/mlx5/devx.c:devx_obj_cleanup() {
	if (obj->flags & DEVX_OBJ_FLAGS_INDIRECT_MKEY &&
	    xa_erase(&obj->ib_dev->odp_mkeys,
		     mlx5_base_mkey(obj->mkey.key)))
		mlx5r_deref_wait_odp_mkey(&obj->mkey);

If uverbs_copy_to() fails on an indirect mkey, devx_obj_cleanup() runs with
obj->mkey still zeroed: if key 0 is present in dev->odp_mkeys, xa_erase()
removes that unrelated entry and mlx5r_deref_wait_odp_mkey() underflows a
zero refcount_t and waits on an uninitialized wait_queue_head_t.  If
devx_handle_mkey_indirect() fails in mlx5r_store_odp_mkey(),
devx_obj_cleanup() likewise erases mlx5_base_mkey(obj->mkey.key) from
dev->odp_mkeys even though storing it failed, reintroducing the regression
fixed by 0da4d48d99df ("IB/mlx5: Fix bad flow upon DEVX mkey creation").

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261111105346.3220427-1-lee@jaeyeong.cc?part=1