scsi: target: Replace all non-returning strlcpy() with strscpy()
authorAzeem Shaikh <azeemshaikh38@gmail.com>
Tue, 16 May 2023 02:53:22 +0000 (02:53 +0000)
committerMartin K. Petersen <martin.petersen@oracle.com>
Wed, 17 May 2023 01:39:44 +0000 (21:39 -0400)
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>
Link: https://lore.kernel.org/r/20230516025322.2804923-1-azeemshaikh38@gmail.com
Reviewed-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
drivers/target/iscsi/iscsi_target_parameters.c
drivers/target/iscsi/iscsi_target_util.c
drivers/target/target_core_configfs.c
drivers/target/target_core_device.c

index 557516c642c3b037e8ffd04393c24b0b37e4a27e..5b90c22ee3dc48a52126f9d6723747870185aa53 100644 (file)
@@ -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,
index 26dc8ed3045b6ad9aee85d543bd2ed144e77d2ed..dc1ac5a0f806559a69727d0c4610056c7e39e931 100644 (file)
@@ -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 =
index 74b67c346dfe9c2e0575368a6e7c11c4f2e861b1..936e5ff1b209ecb9df704766d214326eee5e811d 100644 (file)
@@ -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);
index 90f3f492617242442cd638e25ca360db58f11585..b7ac60f4a21945b1545e7d7112f09efe2e906a69 100644 (file)
@@ -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;