[PATCH] scsi: target: Replace all non-returning strlcpy with strscpy

Azeem Shaikh
2023-05-16T02:53:53Z

strlcpy() reads the entire source buffer first.
This read may exceed the destination size limit.
This is both inefficient and can lead to linear read
overflows if a source string is not NUL-terminated [1].
In an effort to remove strlcpy() completely [2], replace
strlcpy() here with strscpy().
No return values were used, so direct replacement is safe.

[1] https://www.kernel.org/doc/html/latest/process/deprecated.html#strlcpy
[2] https://github.com/KSPP/linux/issues/89

Signed-off-by: Azeem Shaikh <azeemshaikh38@gmail.com>
---
drivers/target/iscsi/iscsi_target_parameters.c | 4 ++--
drivers/target/iscsi/iscsi_target_util.c | 4 ++--
drivers/target/target_core_configfs.c | 10 +++++-----
drivers/target/target_core_device.c | 6 +++---
4 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/target/iscsi/iscsi_target_parameters.c b/drivers/target/iscsi/iscsi_target_parameters.c
index 557516c642c3..5b90c22ee3dc 100644
--- a/drivers/target/iscsi/iscsi_target_parameters.c
+++ b/drivers/target/iscsi/iscsi_target_parameters.c
@@ -726,8 +726,8 @@ static int iscsi_add_notunderstood_response(
}
INIT_LIST_HEAD(&extra_response->er_list);

- strlcpy(extra_response->key, key, sizeof(extra_response->key));
- strlcpy(extra_response->value, NOTUNDERSTOOD,
+ strscpy(extra_response->key, key, sizeof(extra_response->key));
+ strscpy(extra_response->value, NOTUNDERSTOOD,
sizeof(extra_response->value));

list_add_tail(&extra_response->er_list,
diff --git a/drivers/target/iscsi/iscsi_target_util.c b/drivers/target/iscsi/iscsi_target_util.c
index 26dc8ed3045b..dc1ac5a0f806 100644
--- a/drivers/target/iscsi/iscsi_target_util.c
+++ b/drivers/target/iscsi/iscsi_target_util.c
@@ -1321,7 +1321,7 @@ void iscsit_collect_login_stats(
if (conn->param_list)
intrname = iscsi_find_param_from_key(INITIATORNAME,
conn->param_list);
- strlcpy(ls->last_intr_fail_name,
+ strscpy(ls->last_intr_fail_name,
(intrname ? intrname->value : "Unknown"),
sizeof(ls->last_intr_fail_name));

@@ -1360,7 +1360,7 @@ void iscsit_fill_cxn_timeout_err_stats(struct iscsit_session *sess)
return;

spin_lock_bh(&tiqn->sess_err_stats.lock);
- strlcpy(tiqn->sess_err_stats.last_sess_fail_rem_name,
+ strscpy(tiqn->sess_err_stats.last_sess_fail_rem_name,
sess->sess_ops->InitiatorName,
sizeof(tiqn->sess_err_stats.last_sess_fail_rem_name));
tiqn->sess_err_stats.last_sess_failure_type =
diff --git a/drivers/target/target_core_configfs.c b/drivers/target/target_core_configfs.c
index 74b67c346dfe..936e5ff1b209 100644
--- a/drivers/target/target_core_configfs.c
+++ b/drivers/target/target_core_configfs.c
@@ -649,7 +649,7 @@ static void dev_set_t10_wwn_model_alias(struct se_device *dev)
* here without potentially breaking existing setups, so continue to
* truncate one byte shorter than what can be carried in INQUIRY.
*/
- strlcpy(dev->t10_wwn.model, configname, INQUIRY_MODEL_LEN);
+ strscpy(dev->t10_wwn.model, configname, INQUIRY_MODEL_LEN);
}

static ssize_t emulate_model_alias_store(struct config_item *item,
@@ -675,7 +675,7 @@ static ssize_t emulate_model_alias_store(struct config_item *item,
if (flag) {
dev_set_t10_wwn_model_alias(dev);
} else {
- strlcpy(dev->t10_wwn.model, dev->transport->inquiry_prod,
+ strscpy(dev->t10_wwn.model, dev->transport->inquiry_prod,
sizeof(dev->t10_wwn.model));
}
da->emulate_model_alias = flag;
@@ -1426,7 +1426,7 @@ static ssize_t target_wwn_vendor_id_store(struct config_item *item,
}

BUILD_BUG_ON(sizeof(dev->t10_wwn.vendor) != INQUIRY_VENDOR_LEN + 1);
- strlcpy(dev->t10_wwn.vendor, stripped, sizeof(dev->t10_wwn.vendor));
+ strscpy(dev->t10_wwn.vendor, stripped, sizeof(dev->t10_wwn.vendor));

pr_debug("Target_Core_ConfigFS: Set emulated T10 Vendor Identification:"
" %s\n", dev->t10_wwn.vendor);
@@ -1482,7 +1482,7 @@ static ssize_t target_wwn_product_id_store(struct config_item *item,
}

BUILD_BUG_ON(sizeof(dev->t10_wwn.model) != INQUIRY_MODEL_LEN + 1);
- strlcpy(dev->t10_wwn.model, stripped, sizeof(dev->t10_wwn.model));
+ strscpy(dev->t10_wwn.model, stripped, sizeof(dev->t10_wwn.model));

pr_debug("Target_Core_ConfigFS: Set emulated T10 Model Identification: %s\n",
dev->t10_wwn.model);
@@ -1538,7 +1538,7 @@ static ssize_t target_wwn_revision_store(struct config_item *item,
}

BUILD_BUG_ON(sizeof(dev->t10_wwn.revision) != INQUIRY_REVISION_LEN + 1);
- strlcpy(dev->t10_wwn.revision, stripped, sizeof(dev->t10_wwn.revision));
+ strscpy(dev->t10_wwn.revision, stripped, sizeof(dev->t10_wwn.revision));

pr_debug("Target_Core_ConfigFS: Set emulated T10 Revision: %s\n",
dev->t10_wwn.revision);
diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c
index 90f3f4926172..b7ac60f4a219 100644
--- a/drivers/target/target_core_device.c
+++ b/drivers/target/target_core_device.c
@@ -789,10 +789,10 @@ struct se_device *target_alloc_device(struct se_hba *hba, const char *name)
xcopy_lun->lun_tpg = &xcopy_pt_tpg;

/* Preload the default INQUIRY const values */
- strlcpy(dev->t10_wwn.vendor, "LIO-ORG", sizeof(dev->t10_wwn.vendor));
- strlcpy(dev->t10_wwn.model, dev->transport->inquiry_prod,
+ strscpy(dev->t10_wwn.vendor, "LIO-ORG", sizeof(dev->t10_wwn.vendor));
+ strscpy(dev->t10_wwn.model, dev->transport->inquiry_prod,
sizeof(dev->t10_wwn.model));
- strlcpy(dev->t10_wwn.revision, dev->transport->inquiry_rev,
+ strscpy(dev->t10_wwn.revision, dev->transport->inquiry_rev,
sizeof(dev->t10_wwn.revision));

return dev;