Commit 3fe30060 authored by Changpeng Liu's avatar Changpeng Liu
Browse files

nvmf: check HOSTNQN access right for discovery service 



Initiator can use `nvme discover` command to display all
the subsystem's information, because we don't check
the allowed HOSTNQN for Discovery service, so here
adding this feature so that only return the log pages
to the allowed hosts.

Fix issue #576.

Change-Id: I51e6770bd67ea0b41caf9de3a8899923377e6255
Signed-off-by: default avatarChangpeng Liu <changpeng.liu@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/462440


Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: default avatarShuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: default avatarBen Walker <benjamin.walker@intel.com>
Reviewed-by: default avataryidong0635 <dongx.yi@intel.com>
parent 234eb48b

lib/nvmf/ctrlr.c

+2 −1
Original line number Diff line number Diff line
@@ -1491,7 +1491,8 @@ spdk_nvmf_ctrlr_get_log_page(struct spdk_nvmf_request *req) 
	if (subsystem->subtype == SPDK_NVMF_SUBTYPE_DISCOVERY) {

		switch (lid) {

		case SPDK_NVME_LOG_DISCOVERY:

			spdk_nvmf_get_discovery_log_page(subsystem->tgt, req->iov, req->iovcnt, offset, len);

			spdk_nvmf_get_discovery_log_page(subsystem->tgt, ctrlr->hostnqn, req->iov, req->iovcnt, offset,

							 len);

			return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE;

		default:

			goto invalid_log_page;

lib/nvmf/ctrlr_discovery.c

+7 −3
Original line number Diff line number Diff line
@@ -49,7 +49,7 @@ 
#include "spdk_internal/log.h"



static void

nvmf_update_discovery_log(struct spdk_nvmf_tgt *tgt)

nvmf_update_discovery_log(struct spdk_nvmf_tgt *tgt, const char *hostnqn)

{

	uint64_t numrec = 0;

	struct spdk_nvmf_subsystem *subsystem;
@@ -81,6 +81,10 @@ nvmf_update_discovery_log(struct spdk_nvmf_tgt *tgt) 
			continue;

		}



		if (!spdk_nvmf_subsystem_host_allowed(subsystem, hostnqn)) {

			continue;

		}



		for (listener = spdk_nvmf_subsystem_get_first_listener(subsystem); listener != NULL;

		     listener = spdk_nvmf_subsystem_get_next_listener(subsystem, listener)) {

			size_t new_size = cur_size + sizeof(*entry);
@@ -118,7 +122,7 @@ nvmf_update_discovery_log(struct spdk_nvmf_tgt *tgt) 
}



void

spdk_nvmf_get_discovery_log_page(struct spdk_nvmf_tgt *tgt, struct iovec *iov,

spdk_nvmf_get_discovery_log_page(struct spdk_nvmf_tgt *tgt, const char *hostnqn, struct iovec *iov,

				 uint32_t iovcnt, uint64_t offset, uint32_t length)

{

	size_t copy_len = 0;
@@ -127,7 +131,7 @@ spdk_nvmf_get_discovery_log_page(struct spdk_nvmf_tgt *tgt, struct iovec *iov, 


	if (tgt->discovery_log_page == NULL ||

	    tgt->discovery_log_page->genctr != tgt->discovery_genctr) {

		nvmf_update_discovery_log(tgt);

		nvmf_update_discovery_log(tgt, hostnqn);

	}



	/* Copy the valid part of the discovery log page, if any */

lib/nvmf/nvmf_internal.h

+2 −1
Original line number Diff line number Diff line
@@ -375,7 +375,8 @@ int spdk_nvmf_request_complete(struct spdk_nvmf_request *req); 


bool spdk_nvmf_request_get_dif_ctx(struct spdk_nvmf_request *req, struct spdk_dif_ctx *dif_ctx);



void spdk_nvmf_get_discovery_log_page(struct spdk_nvmf_tgt *tgt, struct iovec *iov,

void spdk_nvmf_get_discovery_log_page(struct spdk_nvmf_tgt *tgt, const char *hostnqn,

				      struct iovec *iov,

				      uint32_t iovcnt, uint64_t offset, uint32_t length);



void spdk_nvmf_ctrlr_destruct(struct spdk_nvmf_ctrlr *ctrlr);

test/unit/lib/nvmf/ctrlr.c/ctrlr_ut.c

+2 −1
Original line number Diff line number Diff line
@@ -103,7 +103,8 @@ DEFINE_STUB(spdk_nvmf_ctrlr_write_zeroes_supported, 
	    false);



DEFINE_STUB_V(spdk_nvmf_get_discovery_log_page,

	      (struct spdk_nvmf_tgt *tgt, struct iovec *iov, uint32_t iovcnt, uint64_t offset, uint32_t length));

	      (struct spdk_nvmf_tgt *tgt, const char *hostnqn, struct iovec *iov,

	       uint32_t iovcnt, uint64_t offset, uint32_t length));



DEFINE_STUB(spdk_nvmf_qpair_get_listen_trid,

	    int,

test/unit/lib/nvmf/ctrlr_discovery.c/ctrlr_discovery_ut.c

+7 −5
Original line number Diff line number Diff line
@@ -220,6 +220,7 @@ test_discovery_log(void) 
	/* Add one subsystem and verify that the discovery log contains it */

	subsystem = spdk_nvmf_subsystem_create(&tgt, "nqn.2016-06.io.spdk:subsystem1",

					       SPDK_NVMF_SUBTYPE_NVME, 0);

	subsystem->allow_any_host = true;

	SPDK_CU_ASSERT_FATAL(subsystem != NULL);



	trid.trtype = SPDK_NVME_TRANSPORT_RDMA;
@@ -232,20 +233,21 @@ test_discovery_log(void) 
	/* Get only genctr (first field in the header) */

	memset(buffer, 0xCC, sizeof(buffer));

	disc_log = (struct spdk_nvmf_discovery_log_page *)buffer;

	spdk_nvmf_get_discovery_log_page(&tgt, &iov, 1, 0, sizeof(disc_log->genctr));

	spdk_nvmf_get_discovery_log_page(&tgt, "nqn.2016-06.io.spdk:host1", &iov, 1, 0,

					 sizeof(disc_log->genctr));

	CU_ASSERT(disc_log->genctr == 1); /* one added subsystem */



	/* Get only the header, no entries */

	memset(buffer, 0xCC, sizeof(buffer));

	disc_log = (struct spdk_nvmf_discovery_log_page *)buffer;

	spdk_nvmf_get_discovery_log_page(&tgt, &iov, 1, 0, sizeof(*disc_log));

	spdk_nvmf_get_discovery_log_page(&tgt, "nqn.2016-06.io.spdk:host1", &iov, 1, 0, sizeof(*disc_log));

	CU_ASSERT(disc_log->genctr == 1);

	CU_ASSERT(disc_log->numrec == 1);



	/* Offset 0, exact size match */

	memset(buffer, 0xCC, sizeof(buffer));

	disc_log = (struct spdk_nvmf_discovery_log_page *)buffer;

	spdk_nvmf_get_discovery_log_page(&tgt, &iov, 1, 0,

	spdk_nvmf_get_discovery_log_page(&tgt, "nqn.2016-06.io.spdk:host1", &iov, 1, 0,

					 sizeof(*disc_log) + sizeof(disc_log->entries[0]));

	CU_ASSERT(disc_log->genctr != 0);

	CU_ASSERT(disc_log->numrec == 1);
@@ -254,7 +256,7 @@ test_discovery_log(void) 
	/* Offset 0, oversize buffer */

	memset(buffer, 0xCC, sizeof(buffer));

	disc_log = (struct spdk_nvmf_discovery_log_page *)buffer;

	spdk_nvmf_get_discovery_log_page(&tgt, &iov, 1, 0, sizeof(buffer));

	spdk_nvmf_get_discovery_log_page(&tgt, "nqn.2016-06.io.spdk:host1", &iov, 1, 0, sizeof(buffer));

	CU_ASSERT(disc_log->genctr != 0);

	CU_ASSERT(disc_log->numrec == 1);

	CU_ASSERT(disc_log->entries[0].trtype == 42);
@@ -264,7 +266,7 @@ test_discovery_log(void) 
	/* Get just the first entry, no header */

	memset(buffer, 0xCC, sizeof(buffer));

	entry = (struct spdk_nvmf_discovery_log_page_entry *)buffer;

	spdk_nvmf_get_discovery_log_page(&tgt, &iov,

	spdk_nvmf_get_discovery_log_page(&tgt, "nqn.2016-06.io.spdk:host1", &iov,

					 1,

					 offsetof(struct spdk_nvmf_discovery_log_page, entries[0]),

					 sizeof(*entry));