From b13ee3005dbeaa0297b2921154af726b304286f6 Mon Sep 17 00:00:00 2001 From: Peng Lian Date: Fri, 17 Mar 2023 03:02:24 -0400 Subject: [PATCH] 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: Peng Lian Change-Id: I909d673b5050f21fa193914cc4ffe6634232fa7d Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/17147 Community-CI: Mellanox Build Bot Reviewed-by: Ben Walker Tested-by: SPDK CI Jenkins Reviewed-by: Konrad Sztyber --- lib/nvmf/nvmf.c | 30 ++++++++++++++++++++++-------- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/lib/nvmf/nvmf.c b/lib/nvmf/nvmf.c index 885000acb..4eb24c166 100644 --- a/lib/nvmf/nvmf.c +++ b/lib/nvmf/nvmf.c @@ -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); } }