From 60d645a4e9e7e7ddc20e534fea82aa4e6947f911 Mon Sep 17 00:00:00 2001 From: Dan Carpenter Date: Wed, 15 Jun 2011 10:03:05 -0700 Subject: [PATCH] target: Fix incorrect strlen() NULL terminator checks This patch fixes a number of cases in target core using an incorrectly if (strlen(foo) > SOME_MAX_SIZE) As strlen() returns the number of characters in the string not counting the NULL character at the end. So if you do something like: char buf[10]; if (strlen("0123456789") > 10) return -ETOOLONG; snprintf(buf, 10, "0123456789"); printf("%s\n", buf); then the last "9" gets chopped off and only "012345678" is printed. Plus I threw in one small related cleanup. Signed-off-by: Dan Carpenter Signed-off-by: Nicholas Bellinger --- drivers/target/loopback/tcm_loop.c | 4 ++-- drivers/target/target_core_configfs.c | 22 +++++++++++----------- drivers/target/target_core_device.c | 2 +- drivers/target/target_core_pr.c | 6 +++--- 4 files changed, 17 insertions(+), 17 deletions(-) diff --git a/drivers/target/loopback/tcm_loop.c b/drivers/target/loopback/tcm_loop.c index d4fee2a9d66a..70c2e7fa6664 100644 --- a/drivers/target/loopback/tcm_loop.c +++ b/drivers/target/loopback/tcm_loop.c @@ -1143,7 +1143,7 @@ static ssize_t tcm_loop_tpg_store_nexus( * the fabric protocol_id set in tcm_loop_make_scsi_hba(), and call * tcm_loop_make_nexus() */ - if (strlen(page) > TL_WWN_ADDR_LEN) { + if (strlen(page) >= TL_WWN_ADDR_LEN) { printk(KERN_ERR "Emulated NAA Sas Address: %s, exceeds" " max: %d\n", page, TL_WWN_ADDR_LEN); return -EINVAL; @@ -1324,7 +1324,7 @@ struct se_wwn *tcm_loop_make_scsi_hba( return ERR_PTR(-EINVAL); check_len: - if (strlen(name) > TL_WWN_ADDR_LEN) { + if (strlen(name) >= TL_WWN_ADDR_LEN) { printk(KERN_ERR "Emulated NAA %s Address: %s, exceeds" " max: %d\n", name, tcm_loop_dump_proto_id(tl_hba), TL_WWN_ADDR_LEN); diff --git a/drivers/target/target_core_configfs.c b/drivers/target/target_core_configfs.c index b17abd13c8e3..25c1f49a7d8b 100644 --- a/drivers/target/target_core_configfs.c +++ b/drivers/target/target_core_configfs.c @@ -304,7 +304,7 @@ struct target_fabric_configfs *target_fabric_configfs_init( printk(KERN_ERR "Unable to locate passed fabric name\n"); return NULL; } - if (strlen(name) > TARGET_FABRIC_NAME_SIZE) { + if (strlen(name) >= TARGET_FABRIC_NAME_SIZE) { printk(KERN_ERR "Passed name: %s exceeds TARGET_FABRIC" "_NAME_SIZE\n", name); return NULL; @@ -851,7 +851,7 @@ static ssize_t target_core_dev_wwn_store_attr_vpd_unit_serial( return -EOPNOTSUPP; } - if ((strlen(page) + 1) > INQUIRY_VPD_SERIAL_LEN) { + if (strlen(page) >= INQUIRY_VPD_SERIAL_LEN) { printk(KERN_ERR "Emulated VPD Unit Serial exceeds" " INQUIRY_VPD_SERIAL_LEN: %d\n", INQUIRY_VPD_SERIAL_LEN); return -EOVERFLOW; @@ -917,7 +917,7 @@ static ssize_t target_core_dev_wwn_show_attr_vpd_protocol_identifier( transport_dump_vpd_proto_id(vpd, buf, VPD_TMP_BUF_SIZE); - if ((len + strlen(buf) > PAGE_SIZE)) + if ((len + strlen(buf) >= PAGE_SIZE)) break; len += sprintf(page+len, "%s", buf); @@ -962,19 +962,19 @@ static ssize_t target_core_dev_wwn_show_attr_##_name( \ \ memset(buf, 0, VPD_TMP_BUF_SIZE); \ transport_dump_vpd_assoc(vpd, buf, VPD_TMP_BUF_SIZE); \ - if ((len + strlen(buf) > PAGE_SIZE)) \ + if ((len + strlen(buf) >= PAGE_SIZE)) \ break; \ len += sprintf(page+len, "%s", buf); \ \ memset(buf, 0, VPD_TMP_BUF_SIZE); \ transport_dump_vpd_ident_type(vpd, buf, VPD_TMP_BUF_SIZE); \ - if ((len + strlen(buf) > PAGE_SIZE)) \ + if ((len + strlen(buf) >= PAGE_SIZE)) \ break; \ len += sprintf(page+len, "%s", buf); \ \ memset(buf, 0, VPD_TMP_BUF_SIZE); \ transport_dump_vpd_ident(vpd, buf, VPD_TMP_BUF_SIZE); \ - if ((len + strlen(buf) > PAGE_SIZE)) \ + if ((len + strlen(buf) >= PAGE_SIZE)) \ break; \ len += sprintf(page+len, "%s", buf); \ } \ @@ -1299,7 +1299,7 @@ static ssize_t target_core_dev_pr_show_attr_res_pr_registered_i_pts( &i_buf[0] : "", pr_reg->pr_res_key, pr_reg->pr_res_generation); - if ((len + strlen(buf) > PAGE_SIZE)) + if ((len + strlen(buf) >= PAGE_SIZE)) break; len += sprintf(page+len, "%s", buf); @@ -1496,7 +1496,7 @@ static ssize_t target_core_dev_pr_store_attr_res_aptpl_metadata( ret = -ENOMEM; goto out; } - if (strlen(i_port) > PR_APTPL_MAX_IPORT_LEN) { + if (strlen(i_port) >= PR_APTPL_MAX_IPORT_LEN) { printk(KERN_ERR "APTPL metadata initiator_node=" " exceeds PR_APTPL_MAX_IPORT_LEN: %d\n", PR_APTPL_MAX_IPORT_LEN); @@ -1510,7 +1510,7 @@ static ssize_t target_core_dev_pr_store_attr_res_aptpl_metadata( ret = -ENOMEM; goto out; } - if (strlen(isid) > PR_REG_ISID_LEN) { + if (strlen(isid) >= PR_REG_ISID_LEN) { printk(KERN_ERR "APTPL metadata initiator_isid" "= exceeds PR_REG_ISID_LEN: %d\n", PR_REG_ISID_LEN); @@ -1571,7 +1571,7 @@ static ssize_t target_core_dev_pr_store_attr_res_aptpl_metadata( ret = -ENOMEM; goto out; } - if (strlen(t_port) > PR_APTPL_MAX_TPORT_LEN) { + if (strlen(t_port) >= PR_APTPL_MAX_TPORT_LEN) { printk(KERN_ERR "APTPL metadata target_node=" " exceeds PR_APTPL_MAX_TPORT_LEN: %d\n", PR_APTPL_MAX_TPORT_LEN); @@ -3052,7 +3052,7 @@ static struct config_group *target_core_call_addhbatotarget( int ret; memset(buf, 0, TARGET_CORE_NAME_MAX_LEN); - if (strlen(name) > TARGET_CORE_NAME_MAX_LEN) { + if (strlen(name) >= TARGET_CORE_NAME_MAX_LEN) { printk(KERN_ERR "Passed *name strlen(): %d exceeds" " TARGET_CORE_NAME_MAX_LEN: %d\n", (int)strlen(name), TARGET_CORE_NAME_MAX_LEN); diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c index f8d8af7a0d41..ba698ea62bb2 100644 --- a/drivers/target/target_core_device.c +++ b/drivers/target/target_core_device.c @@ -1431,7 +1431,7 @@ struct se_lun_acl *core_dev_init_initiator_node_lun_acl( struct se_lun_acl *lacl; struct se_node_acl *nacl; - if (strlen(initiatorname) > TRANSPORT_IQN_LEN) { + if (strlen(initiatorname) >= TRANSPORT_IQN_LEN) { printk(KERN_ERR "%s InitiatorName exceeds maximum size.\n", TPG_TFO(tpg)->get_fabric_name()); *ret = -EOVERFLOW; diff --git a/drivers/target/target_core_pr.c b/drivers/target/target_core_pr.c index a79f518ca6e2..b662db3a320b 100644 --- a/drivers/target/target_core_pr.c +++ b/drivers/target/target_core_pr.c @@ -1916,7 +1916,7 @@ static int __core_scsi3_update_aptpl_buf( pr_reg->pr_res_mapped_lun); } - if ((len + strlen(tmp) > pr_aptpl_buf_len)) { + if ((len + strlen(tmp) >= pr_aptpl_buf_len)) { printk(KERN_ERR "Unable to update renaming" " APTPL metadata\n"); spin_unlock(&T10_RES(su_dev)->registration_lock); @@ -1934,7 +1934,7 @@ static int __core_scsi3_update_aptpl_buf( TPG_TFO(tpg)->tpg_get_tag(tpg), lun->lun_sep->sep_rtpi, lun->unpacked_lun, reg_count); - if ((len + strlen(tmp) > pr_aptpl_buf_len)) { + if ((len + strlen(tmp) >= pr_aptpl_buf_len)) { printk(KERN_ERR "Unable to update renaming" " APTPL metadata\n"); spin_unlock(&T10_RES(su_dev)->registration_lock); @@ -1986,7 +1986,7 @@ static int __core_scsi3_write_aptpl_to_file( memset(iov, 0, sizeof(struct iovec)); memset(path, 0, 512); - if (strlen(&wwn->unit_serial[0]) > 512) { + if (strlen(&wwn->unit_serial[0]) >= 512) { printk(KERN_ERR "WWN value for struct se_device does not fit" " into path buffer\n"); return -1; -- 2.20.1