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

iscsi: Use not malloc'ed but fixed size string for target name and alias

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

Maximum size of target name is already defined to be MAX_TARGET_NAME,
and hence use fixed size string whose size is MAX_TARGET_NAME + 1 for
spdk_iscsi_tgt_node::name.

Change psdk_iscsi_tgt_node::alias together.

This change will reduce the potential malloc failure.

Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: Iac4cd6e9d60173ddeb68ca21ce712126c13bc3c4
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/459707
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 10:18:07 +09:00 committed by Darek Stojaczyk
parent 975e48ae8a
commit d1961b5e41
4 changed files with 14 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
lib/iscsi/iscsi.c
View File

@ -1703,7 +1703,7 @@ iscsi_op_login_set_target_info(struct spdk_iscsi_conn *conn,
/* declarative parameters */ /* declarative parameters */
if (target != NULL) { if (target != NULL) {
pthread_mutex_lock(&target->mutex); pthread_mutex_lock(&target->mutex);
if (target->alias != NULL) { if (target->alias[0] != '\0') {
snprintf(buf, sizeof buf, "%s", target->alias); snprintf(buf, sizeof buf, "%s", target->alias);
} else { } else {
snprintf(buf, sizeof buf, "%s", ""); snprintf(buf, sizeof buf, "%s", "");

25
lib/iscsi/tgt_node.c
View File

@ -638,9 +638,6 @@ _iscsi_tgt_node_destruct(void *cb_arg, int rc)
return; return;
} }
free(target->name);
free(target->alias);
pthread_mutex_lock(&g_spdk_iscsi.mutex); pthread_mutex_lock(&g_spdk_iscsi.mutex);
iscsi_tgt_node_delete_all_pg_maps(target); iscsi_tgt_node_delete_all_pg_maps(target);
pthread_mutex_unlock(&g_spdk_iscsi.mutex); pthread_mutex_unlock(&g_spdk_iscsi.mutex);
@ -962,14 +959,12 @@ spdk_iscsi_tgt_node_construct(int target_index,
return NULL; return NULL;
} }
target = malloc(sizeof(*target)); target = calloc(1, sizeof(*target));
if (!target) { if (!target) {
SPDK_ERRLOG("could not allocate target\n"); SPDK_ERRLOG("could not allocate target\n");
return NULL; return NULL;
} }
memset(target, 0, sizeof(*target));
rc = pthread_mutex_init(&target->mutex, NULL); rc = pthread_mutex_init(&target->mutex, NULL);
if (rc != 0) { if (rc != 0) {
SPDK_ERRLOG("tgt_node%d: mutex_init() failed\n", target->num); SPDK_ERRLOG("tgt_node%d: mutex_init() failed\n", target->num);
@ -979,22 +974,14 @@ spdk_iscsi_tgt_node_construct(int target_index,
target->num = target_index; target->num = target_index;
target->name = strdup(fullname); memcpy(target->name, fullname, strlen(fullname));
if (!target->name) {
SPDK_ERRLOG("Could not allocate TargetName\n");
iscsi_tgt_node_destruct(target, NULL, NULL);
return NULL;
}
if (alias == NULL) { if (alias != NULL) {
target->alias = NULL; if (strlen(alias) > MAX_TARGET_NAME) {
} else {
target->alias = strdup(alias);
if (!target->alias) {
SPDK_ERRLOG("Could not allocate TargetAlias\n");
iscsi_tgt_node_destruct(target, NULL, NULL); iscsi_tgt_node_destruct(target, NULL, NULL);
return NULL; return NULL;
} }
memcpy(target->alias, alias, strlen(alias));
} }
target->dev = spdk_scsi_dev_construct(fullname, bdev_name_list, lun_id_list, num_luns, target->dev = spdk_scsi_dev_construct(fullname, bdev_name_list, lun_id_list, num_luns,
@ -1546,7 +1533,7 @@ iscsi_tgt_node_info_json(struct spdk_iscsi_tgt_node *target,
spdk_json_write_named_string(w, "name", target->name); spdk_json_write_named_string(w, "name", target->name);
if (target->alias) { if (target->alias[0] != '\0') {
spdk_json_write_named_string(w, "alias_name", target->alias); spdk_json_write_named_string(w, "alias_name", target->alias);
} }

6
lib/iscsi/tgt_node.h
View File

@ -37,6 +37,8 @@
#include "spdk/stdinc.h" #include "spdk/stdinc.h"
#include "iscsi/iscsi.h"
struct spdk_iscsi_conn; struct spdk_iscsi_conn;
struct spdk_iscsi_init_grp; struct spdk_iscsi_init_grp;
struct spdk_iscsi_portal_grp; struct spdk_iscsi_portal_grp;
@ -62,8 +64,8 @@ struct spdk_iscsi_pg_map {
struct spdk_iscsi_tgt_node { struct spdk_iscsi_tgt_node {
int num; int num;
char *name; char name[MAX_TARGET_NAME + 1];
char *alias; char alias[MAX_TARGET_NAME + 1];
pthread_mutex_t mutex; pthread_mutex_t mutex;

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

@ -376,7 +376,7 @@ node_access_allowed(void)
/* target initialization */ /* target initialization */
memset(&tgtnode, 0, sizeof(struct spdk_iscsi_tgt_node)); memset(&tgtnode, 0, sizeof(struct spdk_iscsi_tgt_node));
tgtnode.name = "iqn.2017-10.spdk.io:0001"; snprintf(tgtnode.name, sizeof(tgtnode.name), "iqn.2017-10.spdk.io:0001");
TAILQ_INIT(&tgtnode.pg_map_head); TAILQ_INIT(&tgtnode.pg_map_head);
memset(&scsi_dev, 0, sizeof(struct spdk_scsi_dev)); memset(&scsi_dev, 0, sizeof(struct spdk_scsi_dev));
@ -438,7 +438,7 @@ node_access_denied_by_empty_netmask(void)
/* target initialization */ /* target initialization */
memset(&tgtnode, 0, sizeof(struct spdk_iscsi_tgt_node)); memset(&tgtnode, 0, sizeof(struct spdk_iscsi_tgt_node));
tgtnode.name = "iqn.2017-10.spdk.io:0001"; snprintf(tgtnode.name, sizeof(tgtnode.name), "iqn.2017-10.spdk.io:0001");
TAILQ_INIT(&tgtnode.pg_map_head); TAILQ_INIT(&tgtnode.pg_map_head);
memset(&scsi_dev, 0, sizeof(struct spdk_scsi_dev)); memset(&scsi_dev, 0, sizeof(struct spdk_scsi_dev));
@ -491,7 +491,7 @@ node_access_multi_initiator_groups_cases(void)
/* target initialization */ /* target initialization */
memset(&tgtnode, 0, sizeof(struct spdk_iscsi_tgt_node)); memset(&tgtnode, 0, sizeof(struct spdk_iscsi_tgt_node));
tgtnode.name = IQN1; snprintf(tgtnode.name, sizeof(tgtnode.name), IQN1);
TAILQ_INIT(&tgtnode.pg_map_head); TAILQ_INIT(&tgtnode.pg_map_head);
memset(&scsi_dev, 0, sizeof(struct spdk_scsi_dev)); memset(&scsi_dev, 0, sizeof(struct spdk_scsi_dev));