Commit b13ee300 authored by Peng Lian's avatar Peng Lian Committed by Konrad Sztyber
Browse files

nvmf: clean sgroup->queued in _nvmf_qpair_destroy when ctrlr is NULL 



Let us consider the following process:
  1. one fabric connect request A comes but the subsystem is paused
     due to adding/removing ns or other operations, so this request A
     will be put into sgroup->queued until the subsystem becomes active;
  2. the subsystem is paused for a long time until the connect timeout,
     related qpair is destroyed, the sgroup->queued will not be cleaned
     because qpair's ctrlr is NULL;
  3. if a new request B comes, it is more likely to be allocated to the
     same memory as the previous fabric command request. And it will be
     put into sgroup->queued again, where has already exists the exactly
     same pointer with request B.

This leads to the pointer hanging problem and it will cause infinitely
loop when traversing sgroup->queued!

So this patch avoids the ptr-hanging problem by checking and cleaning
all sgroups queued req whose qpair is the being destroyed qpair in
_nvmf_qpair_destroy when ctrlr is NULL.

This problem is already described in issue #2133.

Signed-off-by: default avatarPeng <Lian&lt;peng.lian@smartx.com>
Change-Id: I909d673b5050f21fa193914cc4ffe6634232fa7d
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/17147


Community-CI: Mellanox Build Bot
Reviewed-by: default avatarBen Walker <benjamin.walker@intel.com>
Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: default avatarKonrad Sztyber <konrad.sztyber@intel.com>
parent c64ce716

lib/nvmf/nvmf.c

+22 −8
Original line number Diff line number Diff line
@@ -1168,14 +1168,29 @@ spdk_nvmf_poll_group_remove(struct spdk_nvmf_qpair *qpair) 
	qpair->group = NULL;

}



static void

_nvmf_qpair_sgroup_req_clean(struct spdk_nvmf_subsystem_poll_group *sgroup,

			     const struct spdk_nvmf_qpair *qpair)

{

	struct spdk_nvmf_request *req, *tmp;

	TAILQ_FOREACH_SAFE(req, &sgroup->queued, link, tmp) {

		if (req->qpair == qpair) {

			TAILQ_REMOVE(&sgroup->queued, req, link);

			if (nvmf_transport_req_free(req)) {

				SPDK_ERRLOG("Transport request free error!\n");

			}

		}

	}

}



static void

_nvmf_qpair_destroy(void *ctx, int status)

{

	struct nvmf_qpair_disconnect_ctx *qpair_ctx = ctx;

	struct spdk_nvmf_qpair *qpair = qpair_ctx->qpair;

	struct spdk_nvmf_ctrlr *ctrlr = qpair->ctrlr;

	struct spdk_nvmf_request *req, *tmp;

	struct spdk_nvmf_subsystem_poll_group *sgroup;

	uint32_t sid;



	assert(qpair->state == SPDK_NVMF_QPAIR_DEACTIVATING);

	qpair_ctx->qid = qpair->qid;
@@ -1196,13 +1211,12 @@ _nvmf_qpair_destroy(void *ctx, int status) 


	if (ctrlr) {

		sgroup = &qpair->group->sgroups[ctrlr->subsys->id];

		TAILQ_FOREACH_SAFE(req, &sgroup->queued, link, tmp) {

			if (req->qpair == qpair) {

				TAILQ_REMOVE(&sgroup->queued, req, link);

				if (nvmf_transport_req_free(req)) {

					SPDK_ERRLOG("Transport request free error!\n");

				}

			}

		_nvmf_qpair_sgroup_req_clean(sgroup, qpair);

	} else {

		for (sid = 0; sid < qpair->group->num_sgroups; sid++) {

			sgroup = &qpair->group->sgroups[sid];

			assert(sgroup != NULL);

			_nvmf_qpair_sgroup_req_clean(sgroup, qpair);

		}

	}