ivampiresp/Spdk
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

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

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

Maximum size of initiator name is already defined to be
MAX_INITIATOR_NAME, and hence use fixed size string whose size
is MAX_INITIATOR_NAME + 1 for spdk_iscsi_initiator_name::name.

This will also reduce the potential malloc failure.

Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: Ic6bc172125fc6c9c0896499704d2a9b522106da0
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/459705
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
This commit is contained in:
Shuhei Matsumoto 2019-06-28 08:49:28 +09:00 committed by Darek Stojaczyk
parent 1eba9812f2
commit 4a3ad8371c
3 changed files with 31 additions and 38 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
lib/iscsi/init_grp.c
View File

@ -77,13 +77,15 @@ iscsi_init_grp_add_initiator(struct spdk_iscsi_init_grp *ig, char *name)
{ {
struct spdk_iscsi_initiator_name *iname; struct spdk_iscsi_initiator_name *iname;
char *p; char *p;
size_t len;
if (ig->ninitiators >= MAX_INITIATOR) { if (ig->ninitiators >= MAX_INITIATOR) {
SPDK_ERRLOG("> MAX_INITIATOR(=%d) is not allowed\n", MAX_INITIATOR); SPDK_ERRLOG("> MAX_INITIATOR(=%d) is not allowed\n", MAX_INITIATOR);
return -EPERM; return -EPERM;
} }
if (strlen(name) > MAX_INITIATOR_NAME) { len = strlen(name);
if (len > MAX_INITIATOR_NAME) {
SPDK_ERRLOG("Initiator Name is larger than 223 bytes\n"); SPDK_ERRLOG("Initiator Name is larger than 223 bytes\n");
return -EINVAL; return -EINVAL;
} }
@ -93,18 +95,13 @@ iscsi_init_grp_add_initiator(struct spdk_iscsi_init_grp *ig, char *name)
return -EEXIST; return -EEXIST;
} }
iname = malloc(sizeof(*iname)); iname = calloc(1, sizeof(*iname));
if (iname == NULL) { if (iname == NULL) {
SPDK_ERRLOG("malloc() failed for initiator name str\n"); SPDK_ERRLOG("malloc() failed for initiator name str\n");
return -ENOMEM; return -ENOMEM;
} }
iname->name = strdup(name); memcpy(iname->name, name, len);
if (iname->name == NULL) {
SPDK_ERRLOG("strdup() failed for initiator name\n");
free(iname);
return -ENOMEM;
}
/* Replace "ALL" by "ANY" if set */ /* Replace "ALL" by "ANY" if set */
p = strstr(iname->name, "ALL"); p = strstr(iname->name, "ALL");
@ -133,7 +130,6 @@ iscsi_init_grp_delete_initiator(struct spdk_iscsi_init_grp *ig, char *name)
TAILQ_REMOVE(&ig->initiator_head, iname, tailq); TAILQ_REMOVE(&ig->initiator_head, iname, tailq);
ig->ninitiators--; ig->ninitiators--;
free(iname->name);
free(iname); free(iname);
return 0; return 0;
} }
@ -168,7 +164,6 @@ iscsi_init_grp_delete_all_initiators(struct spdk_iscsi_init_grp *ig)
TAILQ_FOREACH_SAFE(iname, &ig->initiator_head, tailq, tmp) { TAILQ_FOREACH_SAFE(iname, &ig->initiator_head, tailq, tmp) {
TAILQ_REMOVE(&ig->initiator_head, iname, tailq); TAILQ_REMOVE(&ig->initiator_head, iname, tailq);
ig->ninitiators--; ig->ninitiators--;
free(iname->name);
free(iname); free(iname);
} }
} }

3
lib/iscsi/init_grp.h
View File

@ -36,9 +36,10 @@
#define SPDK_INIT_GRP_H #define SPDK_INIT_GRP_H
#include "spdk/conf.h" #include "spdk/conf.h"
#include "iscsi/iscsi.h"
struct spdk_iscsi_initiator_name { struct spdk_iscsi_initiator_name {
char *name; char name[MAX_INITIATOR_NAME + 1];
TAILQ_ENTRY(spdk_iscsi_initiator_name) tailq; TAILQ_ENTRY(spdk_iscsi_initiator_name) tailq;
}; };

51
test/unit/lib/iscsi/tgt_node.c/tgt_node_ut.c
View File

@ -365,7 +365,7 @@ node_access_allowed(void)
ig.tag = 1; ig.tag = 1;
ig.ninitiators = 1; ig.ninitiators = 1;
iname.name = "iqn.2017-10.spdk.io:0001"; snprintf(iname.name, sizeof(iname.name), "iqn.2017-10.spdk.io:0001");
TAILQ_INIT(&ig.initiator_head); TAILQ_INIT(&ig.initiator_head);
TAILQ_INSERT_TAIL(&ig.initiator_head, &iname, tailq); TAILQ_INSERT_TAIL(&ig.initiator_head, &iname, tailq);
@ -429,7 +429,7 @@ node_access_denied_by_empty_netmask(void)
ig.tag = 1; ig.tag = 1;
ig.ninitiators = 1; ig.ninitiators = 1;
iname.name = "iqn.2017-10.spdk.io:0001"; snprintf(iname.name, sizeof(iname.name), "iqn.2017-10.spdk.io:0001");
TAILQ_INIT(&ig.initiator_head); TAILQ_INIT(&ig.initiator_head);
TAILQ_INSERT_TAIL(&ig.initiator_head, &iname, tailq); TAILQ_INSERT_TAIL(&ig.initiator_head, &iname, tailq);
@ -482,7 +482,7 @@ node_access_multi_initiator_groups_cases(void)
struct spdk_iscsi_portal_grp pg; struct spdk_iscsi_portal_grp pg;
struct spdk_iscsi_portal portal; struct spdk_iscsi_portal portal;
struct spdk_iscsi_init_grp ig1, ig2; struct spdk_iscsi_init_grp ig1, ig2;
struct spdk_iscsi_initiator_name iname1, iname2; 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_scsi_dev scsi_dev;
struct spdk_iscsi_pg_map *pg_map; struct spdk_iscsi_pg_map *pg_map;
@ -505,7 +505,6 @@ node_access_multi_initiator_groups_cases(void)
TAILQ_INIT(&ig1.netmask_head); TAILQ_INIT(&ig1.netmask_head);
ig1.ninitiators = 1; ig1.ninitiators = 1;
iname1.name = NULL;
TAILQ_INSERT_TAIL(&ig1.initiator_head, &iname1, tailq); TAILQ_INSERT_TAIL(&ig1.initiator_head, &iname1, tailq);
ig1.nnetmasks = 1; ig1.nnetmasks = 1;
@ -518,7 +517,6 @@ node_access_multi_initiator_groups_cases(void)
TAILQ_INIT(&ig2.netmask_head); TAILQ_INIT(&ig2.netmask_head);
ig2.ninitiators = 1; ig2.ninitiators = 1;
iname2.name = NULL;
TAILQ_INSERT_TAIL(&ig2.initiator_head, &iname2, tailq); TAILQ_INSERT_TAIL(&ig2.initiator_head, &iname2, tailq);
ig2.nnetmasks = 1; ig2.nnetmasks = 1;
@ -557,7 +555,7 @@ node_access_multi_initiator_groups_cases(void)
* | denied | - | - | - | denied | * | denied | - | - | - | denied |
* +-------------------------------------------+---------+ * +-------------------------------------------+---------+
*/ */
iname1.name = NO_IQN1; snprintf(iname1.name, sizeof(iname1.name), NO_IQN1);
result = spdk_iscsi_tgt_node_access(&conn, &tgtnode, iqn, addr); result = spdk_iscsi_tgt_node_access(&conn, &tgtnode, iqn, addr);
CU_ASSERT(result == false); CU_ASSERT(result == false);
@ -573,7 +571,7 @@ node_access_multi_initiator_groups_cases(void)
* | allowed | allowed | - | - | allowed | * | allowed | allowed | - | - | allowed |
* +-------------------------------------------+---------+ * +-------------------------------------------+---------+
*/ */
iname1.name = IQN1; snprintf(iname1.name, sizeof(iname1.name), IQN1);
imask1.mask = IP1; imask1.mask = IP1;
result = spdk_iscsi_tgt_node_access(&conn, &tgtnode, iqn, addr); result = spdk_iscsi_tgt_node_access(&conn, &tgtnode, iqn, addr);
@ -590,9 +588,9 @@ node_access_multi_initiator_groups_cases(void)
* | allowed | denied | denied | - | denied | * | allowed | denied | denied | - | denied |
* +-------------------------------------------+---------+ * +-------------------------------------------+---------+
*/ */
iname1.name = IQN1; snprintf(iname1.name, sizeof(iname1.name), IQN1);
imask1.mask = IP2; imask1.mask = IP2;
iname2.name = NO_IQN1; snprintf(iname2.name, sizeof(iname2.name), NO_IQN1);
result = spdk_iscsi_tgt_node_access(&conn, &tgtnode, iqn, addr); result = spdk_iscsi_tgt_node_access(&conn, &tgtnode, iqn, addr);
CU_ASSERT(result == false); CU_ASSERT(result == false);
@ -608,9 +606,9 @@ node_access_multi_initiator_groups_cases(void)
* | allowed | denied | allowed | allowed | allowed | * | allowed | denied | allowed | allowed | allowed |
* +-------------------------------------------+---------+ * +-------------------------------------------+---------+
*/ */
iname1.name = IQN1; snprintf(iname1.name, sizeof(iname1.name), IQN1);
imask1.mask = IP2; imask1.mask = IP2;
iname2.name = IQN1; snprintf(iname2.name, sizeof(iname2.name), IQN1);
imask2.mask = IP1; imask2.mask = IP1;
result = spdk_iscsi_tgt_node_access(&conn, &tgtnode, iqn, addr); result = spdk_iscsi_tgt_node_access(&conn, &tgtnode, iqn, addr);
@ -627,9 +625,9 @@ node_access_multi_initiator_groups_cases(void)
* | allowed | denied | allowed | denied | denied | * | allowed | denied | allowed | denied | denied |
* +---------------------------------------------+---------+ * +---------------------------------------------+---------+
*/ */
iname1.name = IQN1; snprintf(iname1.name, sizeof(iname1.name), IQN1);
imask1.mask = IP2; imask1.mask = IP2;
iname2.name = IQN1; snprintf(iname2.name, sizeof(iname2.name), IQN1);
imask2.mask = IP2; imask2.mask = IP2;
result = spdk_iscsi_tgt_node_access(&conn, &tgtnode, iqn, addr); result = spdk_iscsi_tgt_node_access(&conn, &tgtnode, iqn, addr);
@ -646,9 +644,9 @@ node_access_multi_initiator_groups_cases(void)
* | allowed | denied | not found | - | denied | * | allowed | denied | not found | - | denied |
* +---------------------------------------------+---------+ * +---------------------------------------------+---------+
*/ */
iname1.name = IQN1; snprintf(iname1.name, sizeof(iname1.name), IQN1);
imask1.mask = IP2; imask1.mask = IP2;
iname2.name = IQN2; snprintf(iname2.name, sizeof(iname2.name), IQN2);
result = spdk_iscsi_tgt_node_access(&conn, &tgtnode, iqn, addr); result = spdk_iscsi_tgt_node_access(&conn, &tgtnode, iqn, addr);
CU_ASSERT(result == false); CU_ASSERT(result == false);
@ -664,8 +662,8 @@ node_access_multi_initiator_groups_cases(void)
* | not found | - | denied | - | denied | * | not found | - | denied | - | denied |
* +---------------------------------------------+---------+ * +---------------------------------------------+---------+
*/ */
iname1.name = IQN2; snprintf(iname1.name, sizeof(iname1.name), IQN2);
iname2.name = NO_IQN1; snprintf(iname2.name, sizeof(iname2.name), NO_IQN1);
result = spdk_iscsi_tgt_node_access(&conn, &tgtnode, iqn, addr); result = spdk_iscsi_tgt_node_access(&conn, &tgtnode, iqn, addr);
CU_ASSERT(result == false); CU_ASSERT(result == false);
@ -681,8 +679,8 @@ node_access_multi_initiator_groups_cases(void)
* | not found | - | allowed | allowed | allowed | * | not found | - | allowed | allowed | allowed |
* +---------------------------------------------+---------+ * +---------------------------------------------+---------+
*/ */
iname1.name = IQN2; snprintf(iname1.name, sizeof(iname1.name), IQN2);
iname2.name = IQN1; snprintf(iname2.name, sizeof(iname2.name), IQN1);
imask2.mask = IP1; imask2.mask = IP1;
result = spdk_iscsi_tgt_node_access(&conn, &tgtnode, iqn, addr); result = spdk_iscsi_tgt_node_access(&conn, &tgtnode, iqn, addr);
@ -699,8 +697,8 @@ node_access_multi_initiator_groups_cases(void)
* | not found | - | allowed | denied | denied | * | not found | - | allowed | denied | denied |
* +---------------------------------------------+---------+ * +---------------------------------------------+---------+
*/ */
iname1.name = IQN2; snprintf(iname1.name, sizeof(iname1.name), IQN2);
iname2.name = IQN1; snprintf(iname2.name, sizeof(iname2.name), IQN1);
imask2.mask = IP2; imask2.mask = IP2;
result = spdk_iscsi_tgt_node_access(&conn, &tgtnode, iqn, addr); result = spdk_iscsi_tgt_node_access(&conn, &tgtnode, iqn, addr);
@ -717,8 +715,8 @@ node_access_multi_initiator_groups_cases(void)
* | not found | - | not found | - | denied | * | not found | - | not found | - | denied |
* +---------------------------------------------+---------+ * +---------------------------------------------+---------+
*/ */
iname1.name = IQN2; snprintf(iname1.name, sizeof(iname1.name), IQN2);
iname2.name = IQN2; snprintf(iname2.name, sizeof(iname2.name), IQN2);
result = spdk_iscsi_tgt_node_access(&conn, &tgtnode, iqn, addr); result = spdk_iscsi_tgt_node_access(&conn, &tgtnode, iqn, addr);
CU_ASSERT(result == false); CU_ASSERT(result == false);
@ -734,7 +732,7 @@ allow_iscsi_name_multi_maps_case(void)
struct spdk_iscsi_tgt_node tgtnode; struct spdk_iscsi_tgt_node tgtnode;
struct spdk_iscsi_portal_grp pg1, pg2; struct spdk_iscsi_portal_grp pg1, pg2;
struct spdk_iscsi_init_grp ig; struct spdk_iscsi_init_grp ig;
struct spdk_iscsi_initiator_name iname; struct spdk_iscsi_initiator_name iname = {};
struct spdk_iscsi_pg_map *pg_map1, *pg_map2; struct spdk_iscsi_pg_map *pg_map1, *pg_map2;
struct spdk_scsi_dev scsi_dev; struct spdk_scsi_dev scsi_dev;
char *iqn; char *iqn;
@ -753,7 +751,6 @@ allow_iscsi_name_multi_maps_case(void)
TAILQ_INIT(&ig.initiator_head); TAILQ_INIT(&ig.initiator_head);
ig.ninitiators = 1; ig.ninitiators = 1;
iname.name = NULL;
TAILQ_INSERT_TAIL(&ig.initiator_head, &iname, tailq); TAILQ_INSERT_TAIL(&ig.initiator_head, &iname, tailq);
/* portal group initialization */ /* portal group initialization */
@ -770,12 +767,12 @@ allow_iscsi_name_multi_maps_case(void)
/* test for IG1 <-> PG1, PG2 case */ /* test for IG1 <-> PG1, PG2 case */
iqn = IQN1; iqn = IQN1;
iname.name = IQN1; snprintf(iname.name, sizeof(iname.name), IQN1);
result = iscsi_tgt_node_allow_iscsi_name(&tgtnode, iqn); result = iscsi_tgt_node_allow_iscsi_name(&tgtnode, iqn);
CU_ASSERT(result == true); CU_ASSERT(result == true);
iname.name = IQN2; snprintf(iname.name, sizeof(iname.name), IQN2);
result = iscsi_tgt_node_allow_iscsi_name(&tgtnode, iqn); result = iscsi_tgt_node_allow_iscsi_name(&tgtnode, iqn);
CU_ASSERT(result == false); CU_ASSERT(result == false);