Commit c04b2968 authored by Daniel Verkamp's avatar Daniel Verkamp
Browse files

nvmf: enforce NQN validity at creation time 



Move the NQN validation into the subsytem creation function, and fix the
allowed size to match the spec.

The spec is not clear about the allowed NQN size; for now, interpret it
as 223 bytes, including the null terminator (222 bytes of actual NQN
plus one terminator byte).

Change-Id: If9743ab2fe009d9d852e8b03317d9b38d8af18dc
Signed-off-by: default avatarDaniel Verkamp <daniel.verkamp@intel.com>
parent fd1ba17c

app/nvmf_tgt/conf.c

+0 −30
Original line number Diff line number Diff line
@@ -314,32 +314,6 @@ attach_cb(void *cb_ctx, struct spdk_pci_device *dev, struct spdk_nvme_ctrlr *ctr 
	}

}



static int

spdk_nvmf_validate_nqn(const char *nqn)

{

	size_t len;



	len = strlen(nqn);

	if (len > SPDK_NVMF_NQN_MAX_LEN) {

		SPDK_ERRLOG("Invalid NQN \"%s\": length %zu > max %d\n", nqn, len, SPDK_NVMF_NQN_MAX_LEN);

		return -1;

	}



	if (strncasecmp(nqn, "nqn.", 4) != 0) {

		SPDK_ERRLOG("Invalid NQN \"%s\": NQN must begin with \"nqn.\".\n", nqn);

		return -1;

	}



	/* yyyy-mm. */

	if (!(isdigit(nqn[4]) && isdigit(nqn[5]) && isdigit(nqn[6]) && isdigit(nqn[7]) &&

	      nqn[8] == '-' && isdigit(nqn[9]) && isdigit(nqn[10]) && nqn[11] == '.')) {

		SPDK_ERRLOG("Invalid date code in NQN \"%s\"\n", nqn);

		return -1;

	}



	return 0;

}



static int

spdk_nvmf_allocate_lcore(uint64_t mask, uint32_t lcore)

{
@@ -376,10 +350,6 @@ spdk_nvmf_parse_subsystem(struct spdk_conf_section *sp) 
		return -1;

	}



	if (spdk_nvmf_validate_nqn(nqn) != 0) {

		return -1;

	}



	/* Determine which core to assign to the subsystem */

	mask = spdk_app_get_core_mask();

	lcore = spdk_conf_section_get_intval(sp, "Core");

lib/nvmf/subsystem.c

+30 −0
Original line number Diff line number Diff line
@@ -99,6 +99,32 @@ spdk_nvmf_subsystem_poller(void *arg) 
	spdk_nvmf_subsystem_poll(subsystem);

}



static bool

spdk_nvmf_valid_nqn(const char *nqn)

{

	size_t len;



	len = strlen(nqn);

	if (len >= SPDK_NVMF_NQN_MAX_LEN) {

		SPDK_ERRLOG("Invalid NQN \"%s\": length %zu > max %d\n", nqn, len, SPDK_NVMF_NQN_MAX_LEN - 1);

		return false;

	}



	if (strncasecmp(nqn, "nqn.", 4) != 0) {

		SPDK_ERRLOG("Invalid NQN \"%s\": NQN must begin with \"nqn.\".\n", nqn);

		return false;

	}



	/* yyyy-mm. */

	if (!(isdigit(nqn[4]) && isdigit(nqn[5]) && isdigit(nqn[6]) && isdigit(nqn[7]) &&

	      nqn[8] == '-' && isdigit(nqn[9]) && isdigit(nqn[10]) && nqn[11] == '.')) {

		SPDK_ERRLOG("Invalid date code in NQN \"%s\"\n", nqn);

		return false;

	}



	return true;

}



struct spdk_nvmf_subsystem *

nvmf_create_subsystem(int num, const char *name,

		      enum spdk_nvmf_subtype subtype,
@@ -106,6 +132,10 @@ nvmf_create_subsystem(int num, const char *name, 
{

	struct spdk_nvmf_subsystem	*subsystem;



	if (!spdk_nvmf_valid_nqn(name)) {

		return NULL;

	}



	subsystem = calloc(1, sizeof(struct spdk_nvmf_subsystem));

	if (subsystem == NULL) {

		return NULL;

lib/nvmf/subsystem.h

+1 −2
Original line number Diff line number Diff line
@@ -45,7 +45,6 @@ struct spdk_nvmf_subsystem; 
struct spdk_nvmf_request;

struct nvmf_session;



#define MAX_NQN_SIZE 255

#define MAX_VIRTUAL_NAMESPACE 16



enum spdk_nvmf_subsystem_mode {
@@ -119,7 +118,7 @@ struct spdk_nvmf_controller { 
struct spdk_nvmf_subsystem {

	uint16_t num;

	uint32_t lcore;

	char subnqn[MAX_NQN_SIZE];

	char subnqn[SPDK_NVMF_NQN_MAX_LEN];

	enum spdk_nvmf_subsystem_mode mode;

	enum spdk_nvmf_subtype subtype;

	struct nvmf_session *session;

test/lib/nvmf/subsystem/subsystem_ut.c

+23 −22
Original line number Diff line number Diff line
@@ -109,33 +109,34 @@ spdk_nvmf_session_poll(struct nvmf_session *session) 
static void

nvmf_test_create_subsystem(void)

{



	char correct_name[] = "subsystem1";

	char *nqn_name;

	char nqn[256];

	struct spdk_nvmf_subsystem *subsystem;

	subsystem = nvmf_create_subsystem(1, correct_name, SPDK_NVMF_SUBTYPE_NVME, 1);



	strncpy(nqn, "nqn.2016-06.io.spdk:subsystem1", sizeof(nqn));

	subsystem = nvmf_create_subsystem(1, nqn, SPDK_NVMF_SUBTYPE_NVME, 1);

	SPDK_CU_ASSERT_FATAL(subsystem != NULL);

	CU_ASSERT_EQUAL(subsystem->num, 1);

	CU_ASSERT_EQUAL(subsystem->lcore, 1);

	CU_ASSERT_STRING_EQUAL(subsystem->subnqn, correct_name);



	/* test long name */

	nqn_name = malloc(256);

	memset(nqn_name, '\0', 256);

	memset(nqn_name, 'a', 255);

	subsystem = nvmf_create_subsystem(2, nqn_name, SPDK_NVMF_SUBTYPE_NVME, 2);

	SPDK_CU_ASSERT_FATAL(subsystem != NULL);

	CU_ASSERT_STRING_NOT_EQUAL(subsystem->subnqn, nqn_name);

	CU_ASSERT_EQUAL(strlen(subsystem->subnqn) + 1, MAX_NQN_SIZE);

	free(nqn_name);



	nqn_name = malloc(255);

	memset(nqn_name, '\0', 255);

	memset(nqn_name, 'a', 254);

	subsystem = nvmf_create_subsystem(2, nqn_name, SPDK_NVMF_SUBTYPE_NVME, 2);

	CU_ASSERT_STRING_EQUAL(subsystem->subnqn, nqn);

	nvmf_delete_subsystem(subsystem);



	/* Longest valid name */

	strncpy(nqn, "nqn.2016-06.io.spdk:", sizeof(nqn));

	memset(nqn + strlen(nqn), 'a', 222 - strlen(nqn));

	nqn[222] = '\0';

	CU_ASSERT(strlen(nqn) == 222);

	subsystem = nvmf_create_subsystem(2, nqn, SPDK_NVMF_SUBTYPE_NVME, 2);

	SPDK_CU_ASSERT_FATAL(subsystem != NULL);

	CU_ASSERT_STRING_EQUAL(subsystem->subnqn, nqn_name);

	free(nqn_name);

	CU_ASSERT_STRING_EQUAL(subsystem->subnqn, nqn);

	nvmf_delete_subsystem(subsystem);



	/* Name that is one byte longer than allowed */

	strncpy(nqn, "nqn.2016-06.io.spdk:", sizeof(nqn));

	memset(nqn + strlen(nqn), 'a', 223 - strlen(nqn));

	nqn[223] = '\0';

	CU_ASSERT(strlen(nqn) == 223);

	subsystem = nvmf_create_subsystem(2, nqn, SPDK_NVMF_SUBTYPE_NVME, 2);

	CU_ASSERT(subsystem == NULL);

}



static void