Commit 22532dde authored by Konrad Sztyber's avatar Konrad Sztyber
Browse files

bdev/nvme: take nvme_ctrlr.mutex when setting keys 



Access to nvme_ctrlr.ref should be protected with nvme_ctrlr.mutex,
which was missing in the code responsible for changing DH-HMAC-CHAP
keys.

Additionally, this patch handles the case where nmve_ctrlr.ref = 0,
which can happen when a controller was released, but hasn't been
detached yet.

Reported-by: default avatarMarcin Spiewak <marcin.spiewak@intel.com>
Signed-off-by: default avatarKonrad Sztyber <konrad.sztyber@intel.com>
Change-Id: I3cd28dc612ab3b751bec1d380bf881ed59c30cb5
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/24997


Reviewed-by: default avatarShuhei Matsumoto <smatsumoto@nvidia.com>
Reviewed-by: default avatarJim Harris <jim.harris@samsung.com>
Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
Community-CI: Mellanox Build Bot
parent 55b7c1dc

module/bdev/nvme/bdev_nvme.c

+31 −7
Original line number Diff line number Diff line
@@ -8781,6 +8781,29 @@ nvme_io_path_is_current(struct nvme_io_path *io_path) 
	return current;

}



static struct nvme_ctrlr *

bdev_nvme_next_ctrlr_unsafe(struct nvme_bdev_ctrlr *nbdev_ctrlr, struct nvme_ctrlr *prev)

{

	struct nvme_ctrlr *next;



	/* Must be called under g_bdev_nvme_mutex */

	next = prev != NULL ? TAILQ_NEXT(prev, tailq) : TAILQ_FIRST(&nbdev_ctrlr->ctrlrs);

	while (next != NULL) {

		/* ref can be 0 when the ctrlr was released, but hasn't been detached yet */

		pthread_mutex_lock(&next->mutex);

		if (next->ref > 0) {

			next->ref++;

			pthread_mutex_unlock(&next->mutex);

			return next;

		}



		pthread_mutex_unlock(&next->mutex);

		next = TAILQ_NEXT(next, tailq);

	}



	return NULL;

}



struct bdev_nvme_set_keys_ctx {

	struct nvme_ctrlr	*nctrlr;

	struct spdk_key		*dhchap_key;
@@ -8831,10 +8854,7 @@ bdev_nvme_authenticate_ctrlr_continue(struct bdev_nvme_set_keys_ctx *ctx) 
	struct nvme_ctrlr *next;



	pthread_mutex_lock(&g_bdev_nvme_mutex);

	next = TAILQ_NEXT(ctx->nctrlr, tailq);

	if (next != NULL) {

		next->ref++;

	}

	next = bdev_nvme_next_ctrlr_unsafe(NULL, ctx->nctrlr);

	pthread_mutex_unlock(&g_bdev_nvme_mutex);



	nvme_ctrlr_release(ctx->nctrlr);
@@ -8963,9 +8983,13 @@ bdev_nvme_set_keys(const char *name, const char *dhchap_key, const char *dhchap_ 
		bdev_nvme_free_set_keys_ctx(ctx);

		return -ENODEV;

	}

	assert(!TAILQ_EMPTY(&nbdev_ctrlr->ctrlrs));

	nctrlr = TAILQ_FIRST(&nbdev_ctrlr->ctrlrs);

	nctrlr->ref++;

	nctrlr = bdev_nvme_next_ctrlr_unsafe(nbdev_ctrlr, NULL);

	if (nctrlr == NULL) {

		SPDK_ERRLOG("Could not find any nvme_ctrlrs on bdev_ctrlr %s\n", name);

		pthread_mutex_unlock(&g_bdev_nvme_mutex);

		bdev_nvme_free_set_keys_ctx(ctx);

		return -ENODEV;

	}

	pthread_mutex_unlock(&g_bdev_nvme_mutex);



	ctx->nctrlr = nctrlr;