scsi: qla2xxx: Replace all non-returning strlcpy with strscpy

PATCH Applied Azeem Shaikh 3 participants 6 messages May 16, 2023 – May 22, 2023

Subsystems (2)

SCSI QLogic QLA2XXX FC-SCSI driver

lore

Share & tools

lore

Azeem Shaikh

author

2023-05-16 02:54 UTC #0 lore raw

PATCH 1/1 scsi: qla2xxx: Replace all non-returning strlcpy with strscpy

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 <[email protected]>

---
 drivers/scsi/qla2xxx/qla_init.c |    8 ++++----
 drivers/scsi/qla2xxx/qla_mr.c   |   20 ++++++++++----------
 2 files changed, 14 insertions(+), 14 deletions(-)

drivers/scsi/qla2xxx/qla_init.c +4 -4

index ec0423ec6681..e831d14a4044 100644
@@ -4861,7 +4861,7 @@ qla2x00_set_model_info(scsi_qla_host_t *vha, uint8_t *model, size_t len,
 		if (use_tbl &&
 		    ha->pdev->subsystem_vendor == PCI_VENDOR_ID_QLOGIC &&
 		    index < QLA_MODEL_NAMES)
-			strlcpy(ha->model_desc,
+			strscpy(ha->model_desc,
 			    qla2x00_model_name[index * 2 + 1],
 			    sizeof(ha->model_desc));
 	} else {

@@ -4869,14 +4869,14 @@ qla2x00_set_model_info(scsi_qla_host_t *vha, uint8_t *model, size_t len,
 		if (use_tbl &&
 		    ha->pdev->subsystem_vendor == PCI_VENDOR_ID_QLOGIC &&
 		    index < QLA_MODEL_NAMES) {
-			strlcpy(ha->model_number,
+			strscpy(ha->model_number,
 				qla2x00_model_name[index * 2],
 				sizeof(ha->model_number));
-			strlcpy(ha->model_desc,
+			strscpy(ha->model_desc,
 			    qla2x00_model_name[index * 2 + 1],
 			    sizeof(ha->model_desc));
 		} else {
-			strlcpy(ha->model_number, def,
+			strscpy(ha->model_number, def,
 				sizeof(ha->model_number));
 		}
 	}

drivers/scsi/qla2xxx/qla_mr.c +10 ~~-10~~

index f726eb8449c5..083f94e43fba 100644
@@ -691,7 +691,7 @@ qlafx00_pci_info_str(struct scsi_qla_host *vha, char *str, size_t str_len)
 	struct qla_hw_data *ha = vha->hw;
 
 	if (pci_is_pcie(ha->pdev))
-		strlcpy(str, "PCIe iSA", str_len);
+		strscpy(str, "PCIe iSA", str_len);
 	return str;
 }

@@ -1850,21 +1850,21 @@ qlafx00_fx_disc(scsi_qla_host_t *vha, fc_port_t *fcport, uint16_t fx_type)
 			phost_info = &preg_hsi->hsi;
 			memset(preg_hsi, 0, sizeof(struct register_host_info));
 			phost_info->os_type = OS_TYPE_LINUX;
-			strlcpy(phost_info->sysname, p_sysid->sysname,
+			strscpy(phost_info->sysname, p_sysid->sysname,
 				sizeof(phost_info->sysname));
-			strlcpy(phost_info->nodename, p_sysid->nodename,
+			strscpy(phost_info->nodename, p_sysid->nodename,
 				sizeof(phost_info->nodename));
 			if (!strcmp(phost_info->nodename, "(none)"))
 				ha->mr.host_info_resend = true;
-			strlcpy(phost_info->release, p_sysid->release,
+			strscpy(phost_info->release, p_sysid->release,
 				sizeof(phost_info->release));
-			strlcpy(phost_info->version, p_sysid->version,
+			strscpy(phost_info->version, p_sysid->version,
 				sizeof(phost_info->version));
-			strlcpy(phost_info->machine, p_sysid->machine,
+			strscpy(phost_info->machine, p_sysid->machine,
 				sizeof(phost_info->machine));
-			strlcpy(phost_info->domainname, p_sysid->domainname,
+			strscpy(phost_info->domainname, p_sysid->domainname,
 				sizeof(phost_info->domainname));
-			strlcpy(phost_info->hostdriver, QLA2XXX_VERSION,
+			strscpy(phost_info->hostdriver, QLA2XXX_VERSION,
 				sizeof(phost_info->hostdriver));
 			preg_hsi->utc = (uint64_t)ktime_get_real_seconds();
 			ql_dbg(ql_dbg_init, vha, 0x0149,

@@ -1909,9 +1909,9 @@ qlafx00_fx_disc(scsi_qla_host_t *vha, fc_port_t *fcport, uint16_t fx_type)
 	if (fx_type == FXDISC_GET_CONFIG_INFO) {
 		struct config_info_data *pinfo =
 		    (struct config_info_data *) fdisc->u.fxiocb.rsp_addr;
-		strlcpy(vha->hw->model_number, pinfo->model_num,
+		strscpy(vha->hw->model_number, pinfo->model_num,
 			ARRAY_SIZE(vha->hw->model_number));
-		strlcpy(vha->hw->model_desc, pinfo->model_description,
+		strscpy(vha->hw->model_desc, pinfo->model_description,
 			ARRAY_SIZE(vha->hw->model_desc));
 		memcpy(&vha->hw->mr.symbolic_name, pinfo->symbolic_name,
 		    sizeof(vha->hw->mr.symbolic_name));

Kees Cook

Reviewed-by on v1 1/1

2023-05-16 18:18 UTC #1 lore raw

Quoted 8 lines · Azeem Shaikh

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.

Quoted 4 lines

[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 <[email protected]>

Reviewed-by Kees Cook <[email protected]>

Signature · 1 line

Kees Cook

Martin K. Petersen

Applied

2023-05-17 01:42 UTC #2 lore raw

In reply to Azeem Shaikh

Azeem,

Quoted 6 lines

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.

Applied to 6.5/scsi-staging, thanks!

Signature · 1 line

Martin K. Petersen	Oracle Linux Engineering

Azeem Shaikh

author

2023-05-17 14:11 UTC #3 lore raw

Quoted 4 lines · Martin K. Petersen

Azeem,

Quoted 9 lines

Quoted 6 lines
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.
Applied to 6.5/scsi-staging, thanks!

Thanks a lot for the quick response Martin (on this and other patches too). Just for my understanding, do you mind pointing me to the 6.5/scsi-staging tree?

Azeem Shaikh

author

2023-05-18 04:48 UTC #4 lore raw

Quoted 14 lines · Azeem Shaikh

Quoted 11 lines · Martin K. Petersen
Azeem,
Quoted 6 lines
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.

Quoted 6 lines

Quoted 2 lines
Applied to 6.5/scsi-staging, thanks!
Thanks a lot for the quick response Martin (on this and other patches too). Just for my understanding, do you mind pointing me to the 6.5/scsi-staging tree?

Found it: https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git/log/?h=6.5/scsi-staging

Martin K. Petersen

Applied

2023-05-22 22:46 UTC #5 lore raw

In reply to Azeem Shaikh

Quoted 9 lines · Azeem Shaikh

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.
[...]

Applied to 6.5/scsi-queue, thanks!

[1/1] scsi: qla2xxx: Replace all non-returning strlcpy with strscpy

      https://git.kernel.org/mkp/scsi/c/37f1663c9193

Signature · 1 line

Martin K. Petersen	Oracle Linux Engineering