RDMA/mlx5: question about a failed destroy in the SRQ create error path

drivers/infiniband/hw/mlx5/srq.c int mlx5_ib_create_srq(struct ib_srq *ib_srq, · line 321

[Severity: High]
If mlx5_cmd_destroy_srq() fails here and mlx5_ib_create_srq() untracks the SRQ and returns err without calling destroy_srq_user(), doesn't that orphan the live firmware SRQ without any kernel handle to retry destroying it once the dependent DEVX object is cleaned up, blocking destruction of its parent objects and leaving a dangling entry on context->db_page_list?

When mlx5_cmd_create_srq() creates the firmware SRQ in mlx5_ib_create_srq(), the hardware resource is bound to in.pd, in.cqn (either the user CQ or the device-global kernel CQ dev->devr.c0 for IB_SRQT_BASIC), and in.xrcd (either the user XRCD or dev->devr.xrcdn0).

When mlx5_ib_create_srq() returns an error to ib_create_srq_user(), the RDMA core assumes the hardware SRQ was destroyed, decrements usecnt on the parent objects, and frees srq:

drivers/infiniband/core/verbs.c:ib_create_srq_user() {
	ret = pd->device->ops.create_srq(srq, srq_init_attr, udata);
	if (ret) {
		rdma_restrack_put(&srq->res);
		atomic_dec(&pd->usecnt);
		if (srq->srq_type == IB_SRQT_XRC && srq->ext.xrc.xrcd)
			atomic_dec(&srq->ext.xrc.xrcd->usecnt);
		if (ib_srq_has_cq(srq->srq_type))
			atomic_dec(&srq->ext.cq->usecnt);
		kfree(srq);
		return ERR_PTR(ret);
	}
}

Because the ib_uobject was never initialized with the SRQ and mlx5_cmd_untrack_srq() removed srq->msrq from dev->srq_table, nothing in the kernel will ever re-issue the firmware destroy command for srq->msrq.srqn, even after the dependent DEVX object is destroyed.

As a result, the firmware SRQ stays bound to its PD, CQ, and XRCD forever. When the user closes the file descriptor, uverbs_destroy_ufile_hw() will try to destroy those parent objects after cleaning up DEVX objects, which will fail in firmware and trigger the cleanup warning and forced teardown:

drivers/infiniband/core/uverbs_main.c:uverbs_destroy_ufile_hw() {
	while (!list_empty(&ufile->uobjects) &&
	       !__uverbs_cleanup_ufile(ufile, reason)) {
	}

	if (WARN_ON(!list_empty(&ufile->uobjects)))
		__uverbs_cleanup_ufile(ufile, RDMA_REMOVE_DRIVER_FAILURE);
}

For IB_SRQT_BASIC SRQs, the orphaned firmware SRQ also remains bound to dev->devr.c0 and dev->devr.xrcdn0, causing ib_destroy_cq(devr->c0) to fail during device teardown in mlx5_ib_dev_res_cleanup().

In addition, skipping destroy_srq_user() skips mlx5_ib_db_unmap_user() and ib_umem_release(srq->umem). During create_srq_user(), mlx5_ib_db_map_user_desc() allocates a struct mlx5_ib_user_db_page, grabs current->mm, and links page->list onto &context->db_page_list:

drivers/infiniband/hw/mlx5/doorbell.c:mlx5_ib_db_map_user_desc() {
	page->umem = umem;
	if (page->desc.type == IB_UVERBS_BUFFER_TYPE_VA) {
		mmgrab(current->mm);
		page->mm = current->mm;
	}
	list_add(&page->list, &context->db_page_list);

found:

	db->dma = sg_dma_address(page->umem->sgt_append.sgt.sgl) +
		  (desc->addr & ~PAGE_MASK);
	db->u.user_page = page;
	++page->refcnt;
}

Because mlx5_ib_dealloc_ucontext() does not drain context->db_page_list, skipping mlx5_ib_db_unmap_user() leaves page->list linked to context->db_page_list after ufile_destroy_ucontext() frees context, while permanently leaking srq->umem, page->umem, pinned user pages, and the mmgrab(current->mm) reference.

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1