Commit 7d1db86f authored by Jim Harris's avatar Jim Harris
Browse files

iscsi: properly handle partial keys 



This includes properly detecting when a key's name
extends past the end of the valid data.

Note that the unit tests were using sizeof() instead
of strlen() since some of the strings contain
NULL characters.  This means that we should be
subtracting one to account for the implicit null
character at the end of the string.  Note that the
iSCSI spec only says that the key/value pair has to
end with a null character - a key/value pair that
is split across two PDUs will not have a NULL character
at the end of the first PDU.

Signed-off-by: default avatarJim Harris <james.r.harris@intel.com>
Change-Id: Ie95d6dd3b9ffa6a3902a31771ac4edb482418cce

Reviewed-on: https://review.gerrithub.io/c/442450


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

lib/iscsi/param.c

+23 −5
Original line number Diff line number Diff line
@@ -223,7 +223,9 @@ spdk_iscsi_parse_param(struct iscsi_param **params, const uint8_t *data, uint32_ 
	int key_len, val_len;

	int max_len;



	key_end = strchr(data, '=');

	data_len = strnlen(data, data_len);

	/* No such thing as strnchr so use memchr instead. */

	key_end = memchr(data, '=', data_len);

	if (!key_end) {

		SPDK_ERRLOG("'=' not found\n");

		return -1;
@@ -343,13 +345,29 @@ spdk_iscsi_parse_params(struct iscsi_param **params, const uint8_t *data, 


		/*

		 * reverse iterate the string from the tail not including '\0'

		 * index of last '\0' is len -1.

		 */

		for (i = len - 2; data[i] != '\0' && i > 0; i--) {

		for (i = len - 1; data[i] != '\0' && i > 0; i--) {

			;

		}

		*partial_parameter = xstrdup(&data[i == 0 ? 0 : i + 1]);

		len = (i == 0 ? 0 : i + 1);

		if (i != 0) {

			/* We found a NULL character - don't copy it into the

			 * partial parameter.

			 */

			i++;

		}



		*partial_parameter = calloc(1, len - i + 1);

		if (*partial_parameter == NULL) {

			SPDK_ERRLOG("could not allocate partial parameter\n");

			return -1;

		}

		memcpy(*partial_parameter, &data[i], len - i);

		if (i == 0) {

			/* No full parameters to parse - so return now. */

			return 0;

		} else {

			len = i - 1;

		}

	}



	while (offset < len && data[offset] != '\0') {

test/unit/lib/iscsi/param.c/param_ut.c

+13 −1
Original line number Diff line number Diff line
@@ -188,7 +188,7 @@ list_negotiation_test(void) 


#define PARSE(strconst, partial_enabled, partial_text) \

	data = strconst; \

	len = sizeof(strconst); \

	len = sizeof(strconst) - 1; \

	rc = spdk_iscsi_parse_params(&params, data, len, partial_enabled, partial_text)



#define EXPECT_VAL(key, expected_value) \
@@ -274,6 +274,18 @@ parse_valid_test(void) 
	EXPECT_VAL("OOOOLL", "MMMM");

	CU_ASSERT_PTR_NULL(partial_parameter);



	partial_parameter = NULL;

	data = "PartialKey=";

	len = 7;

	rc = spdk_iscsi_parse_params(&params, data, len, true, &partial_parameter);

	CU_ASSERT(rc == 0);

	CU_ASSERT_STRING_EQUAL(partial_parameter, "Partial");

	EXPECT_NULL("PartialKey");

	PARSE("Key=Value", false, &partial_parameter);

	CU_ASSERT(rc == 0);

	EXPECT_VAL("PartialKey", "Value");

	CU_ASSERT_PTR_NULL(partial_parameter);



	spdk_iscsi_param_free(params);

}