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

[sashiko-bot@kernel.org]

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