Commit 975e48ae authored by Shuhei Matsumoto's avatar Shuhei Matsumoto Committed by Darek Stojaczyk
Browse files

iscsi: Use not malloc'ed but fixed size string for initiator address 



Using malloc'ed string for string in iSCSI target has caused
scan-build error.

Maximum size of initiator address is already defined to be
MAX_INITIATOR_ADDR, and hence use fixed size string whose size
is MAX_INITIATOR_ADDR + 1 for spdk_iscsi_initiator_mask::mask.

This change will reduce the potential malloc failure.

Signed-off-by: default avatarShuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: Ic39e08986c9377800ce58a1cb5b8401c6b71cf96
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/459706


Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: default avatarBen Walker <benjamin.walker@intel.com>
Reviewed-by: default avatarDarek Stojaczyk <dariusz.stojaczyk@intel.com>
parent 4a3ad837

lib/iscsi/init_grp.c

+9 −9
Original line number Diff line number Diff line
@@ -211,29 +211,31 @@ iscsi_init_grp_add_netmask(struct spdk_iscsi_init_grp *ig, char *mask) 
{

	struct spdk_iscsi_initiator_netmask *imask;

	char *p;

	size_t len;



	if (ig->nnetmasks >= MAX_NETMASK) {

		SPDK_ERRLOG("> MAX_NETMASK(=%d) is not allowed\n", MAX_NETMASK);

		return -EPERM;

	}



	len = strlen(mask);

	if (len > MAX_INITIATOR_ADDR) {

		SPDK_ERRLOG("Initiator Name is larger than %d bytes\n", MAX_INITIATOR_ADDR);

		return -EINVAL;

	}



	imask = iscsi_init_grp_find_netmask(ig, mask);

	if (imask != NULL) {

		return -EEXIST;

	}



	imask = malloc(sizeof(*imask));

	imask = calloc(1, sizeof(*imask));

	if (imask == NULL) {

		SPDK_ERRLOG("malloc() failed for inititator mask str\n");

		return -ENOMEM;

	}



	imask->mask = strdup(mask);

	if (imask->mask == NULL) {

		SPDK_ERRLOG("strdup() failed for initiator mask\n");

		free(imask);

		return -ENOMEM;

	}

	memcpy(imask->mask, mask, len);



	/* Replace "ALL" by "ANY" if set */

	p = strstr(imask->mask, "ALL");
@@ -262,7 +264,6 @@ iscsi_init_grp_delete_netmask(struct spdk_iscsi_init_grp *ig, char *mask) 


	TAILQ_REMOVE(&ig->netmask_head, imask, tailq);

	ig->nnetmasks--;

	free(imask->mask);

	free(imask);

	return 0;

}
@@ -296,7 +297,6 @@ iscsi_init_grp_delete_all_netmasks(struct spdk_iscsi_init_grp *ig) 
	TAILQ_FOREACH_SAFE(imask, &ig->netmask_head, tailq, tmp) {

		TAILQ_REMOVE(&ig->netmask_head, imask, tailq);

		ig->nnetmasks--;

		free(imask->mask);

		free(imask);

	}

}

lib/iscsi/init_grp.h

+2 −1
Original line number Diff line number Diff line
@@ -37,6 +37,7 @@ 


#include "spdk/conf.h"

#include "iscsi/iscsi.h"

#include "iscsi/conn.h"



struct spdk_iscsi_initiator_name {

	char name[MAX_INITIATOR_NAME + 1];
@@ -44,7 +45,7 @@ struct spdk_iscsi_initiator_name { 
};



struct spdk_iscsi_initiator_netmask {

	char *mask;

	char mask[MAX_INITIATOR_ADDR + 1];

	TAILQ_ENTRY(spdk_iscsi_initiator_netmask) tailq;

};

test/unit/lib/iscsi/tgt_node.c/tgt_node_ut.c

+11 −13
Original line number Diff line number Diff line
@@ -370,7 +370,7 @@ node_access_allowed(void) 
	TAILQ_INSERT_TAIL(&ig.initiator_head, &iname, tailq);



	ig.nnetmasks = 1;

	imask.mask = "192.168.2.0/24";

	snprintf(imask.mask, sizeof(imask.mask), "192.168.2.0/24");

	TAILQ_INIT(&ig.netmask_head);

	TAILQ_INSERT_TAIL(&ig.netmask_head, &imask, tailq);
@@ -483,7 +483,7 @@ node_access_multi_initiator_groups_cases(void) 
	struct spdk_iscsi_portal portal;

	struct spdk_iscsi_init_grp ig1, ig2;

	struct spdk_iscsi_initiator_name iname1 = {}, iname2 = {};

	struct spdk_iscsi_initiator_netmask imask1, imask2;

	struct spdk_iscsi_initiator_netmask imask1 = {}, imask2 = {};

	struct spdk_scsi_dev scsi_dev;

	struct spdk_iscsi_pg_map *pg_map;

	char *iqn, *addr;
@@ -508,7 +508,6 @@ node_access_multi_initiator_groups_cases(void) 
	TAILQ_INSERT_TAIL(&ig1.initiator_head, &iname1, tailq);



	ig1.nnetmasks = 1;

	imask1.mask = NULL;

	TAILQ_INSERT_TAIL(&ig1.netmask_head, &imask1, tailq);



	memset(&ig2, 0, sizeof(struct spdk_iscsi_init_grp));
@@ -520,7 +519,6 @@ node_access_multi_initiator_groups_cases(void) 
	TAILQ_INSERT_TAIL(&ig2.initiator_head, &iname2, tailq);



	ig2.nnetmasks = 1;

	imask2.mask = NULL;

	TAILQ_INSERT_TAIL(&ig2.netmask_head, &imask2, tailq);



	/* portal group initialization */
@@ -572,7 +570,7 @@ node_access_multi_initiator_groups_cases(void) 
	 * +-------------------------------------------+---------+

	 */

	snprintf(iname1.name, sizeof(iname1.name), IQN1);

	imask1.mask = IP1;

	snprintf(imask1.mask, sizeof(imask1.mask), IP1);



	result = spdk_iscsi_tgt_node_access(&conn, &tgtnode, iqn, addr);

	CU_ASSERT(result == true);
@@ -589,7 +587,7 @@ node_access_multi_initiator_groups_cases(void) 
	 * +-------------------------------------------+---------+

	 */

	snprintf(iname1.name, sizeof(iname1.name), IQN1);

	imask1.mask = IP2;

	snprintf(imask1.mask, sizeof(imask1.mask), IP2);

	snprintf(iname2.name, sizeof(iname2.name), NO_IQN1);



	result = spdk_iscsi_tgt_node_access(&conn, &tgtnode, iqn, addr);
@@ -607,9 +605,9 @@ node_access_multi_initiator_groups_cases(void) 
	 * +-------------------------------------------+---------+

	 */

	snprintf(iname1.name, sizeof(iname1.name), IQN1);

	imask1.mask = IP2;

	snprintf(imask1.mask, sizeof(imask1.mask), IP2);

	snprintf(iname2.name, sizeof(iname2.name), IQN1);

	imask2.mask = IP1;

	snprintf(imask2.mask, sizeof(imask2.mask), IP1);



	result = spdk_iscsi_tgt_node_access(&conn, &tgtnode, iqn, addr);

	CU_ASSERT(result == true);
@@ -626,9 +624,9 @@ node_access_multi_initiator_groups_cases(void) 
	 * +---------------------------------------------+---------+

	 */

	snprintf(iname1.name, sizeof(iname1.name), IQN1);

	imask1.mask = IP2;

	snprintf(imask1.mask, sizeof(imask1.mask), IP2);

	snprintf(iname2.name, sizeof(iname2.name), IQN1);

	imask2.mask = IP2;

	snprintf(imask2.mask, sizeof(imask2.mask), IP2);



	result = spdk_iscsi_tgt_node_access(&conn, &tgtnode, iqn, addr);

	CU_ASSERT(result == false);
@@ -645,7 +643,7 @@ node_access_multi_initiator_groups_cases(void) 
	 * +---------------------------------------------+---------+

	 */

	snprintf(iname1.name, sizeof(iname1.name), IQN1);

	imask1.mask = IP2;

	snprintf(imask1.mask, sizeof(imask1.mask), IP2);

	snprintf(iname2.name, sizeof(iname2.name), IQN2);



	result = spdk_iscsi_tgt_node_access(&conn, &tgtnode, iqn, addr);
@@ -681,7 +679,7 @@ node_access_multi_initiator_groups_cases(void) 
	 */

	snprintf(iname1.name, sizeof(iname1.name), IQN2);

	snprintf(iname2.name, sizeof(iname2.name), IQN1);

	imask2.mask = IP1;

	snprintf(imask2.mask, sizeof(imask2.mask), IP1);



	result = spdk_iscsi_tgt_node_access(&conn, &tgtnode, iqn, addr);

	CU_ASSERT(result == true);
@@ -699,7 +697,7 @@ node_access_multi_initiator_groups_cases(void) 
	 */

	snprintf(iname1.name, sizeof(iname1.name), IQN2);

	snprintf(iname2.name, sizeof(iname2.name), IQN1);

	imask2.mask = IP2;

	snprintf(imask2.mask, sizeof(imask2.mask), IP2);



	result = spdk_iscsi_tgt_node_access(&conn, &tgtnode, iqn, addr);

	CU_ASSERT(result == false);