* [PATCH] scsi: bfa: Replace all non-returning strlcpy with strscpy
@ 2023-05-16 1:33 Azeem Shaikh
2023-05-16 2:02 ` Azeem Shaikh
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Azeem Shaikh @ 2023-05-16 1:33 UTC (permalink / raw)
To: Anil Gurumurthy, Sudarsana Kalluru
Cc: azeems, Azeem Shaikh, linux-scsi, linux-kernel,
James E.J. Bottomley, Martin K. Petersen
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/scsi/bfa/bfa_fcbuild.c | 4 ++--
drivers/scsi/bfa/bfa_fcs.c | 4 ++--
drivers/scsi/bfa/bfa_fcs_lport.c | 20 ++++++++++----------
drivers/scsi/bfa/bfa_ioc.c | 2 +-
drivers/scsi/bfa/bfa_svc.c | 2 +-
drivers/scsi/bfa/bfad.c | 10 +++++-----
drivers/scsi/bfa/bfad_attr.c | 2 +-
drivers/scsi/bfa/bfad_bsg.c | 4 ++--
drivers/scsi/bfa/bfad_im.c | 2 +-
9 files changed, 25 insertions(+), 25 deletions(-)
diff --git a/drivers/scsi/bfa/bfa_fcbuild.c b/drivers/scsi/bfa/bfa_fcbuild.c
index df18d9d2af53..773c84af784c 100644
--- a/drivers/scsi/bfa/bfa_fcbuild.c
+++ b/drivers/scsi/bfa/bfa_fcbuild.c
@@ -1134,7 +1134,7 @@ fc_rspnid_build(struct fchs_s *fchs, void *pyld, u32 s_id, u16 ox_id,
memset(rspnid, 0, sizeof(struct fcgs_rspnid_req_s));
rspnid->dap = s_id;
- strlcpy(rspnid->spn, name, sizeof(rspnid->spn));
+ strscpy(rspnid->spn, name, sizeof(rspnid->spn));
rspnid->spn_len = (u8) strlen(rspnid->spn);
return sizeof(struct fcgs_rspnid_req_s) + sizeof(struct ct_hdr_s);
@@ -1155,7 +1155,7 @@ fc_rsnn_nn_build(struct fchs_s *fchs, void *pyld, u32 s_id,
memset(rsnn_nn, 0, sizeof(struct fcgs_rsnn_nn_req_s));
rsnn_nn->node_name = node_name;
- strlcpy(rsnn_nn->snn, name, sizeof(rsnn_nn->snn));
+ strscpy(rsnn_nn->snn, name, sizeof(rsnn_nn->snn));
rsnn_nn->snn_len = (u8) strlen(rsnn_nn->snn);
return sizeof(struct fcgs_rsnn_nn_req_s) + sizeof(struct ct_hdr_s);
diff --git a/drivers/scsi/bfa/bfa_fcs.c b/drivers/scsi/bfa/bfa_fcs.c
index d2d396ca0e9a..5023c0ab4277 100644
--- a/drivers/scsi/bfa/bfa_fcs.c
+++ b/drivers/scsi/bfa/bfa_fcs.c
@@ -761,7 +761,7 @@ bfa_fcs_fabric_psymb_init(struct bfa_fcs_fabric_s *fabric)
bfa_ioc_get_adapter_model(&fabric->fcs->bfa->ioc, model);
/* Model name/number */
- strlcpy(port_cfg->sym_name.symname, model,
+ strscpy(port_cfg->sym_name.symname, model,
BFA_SYMNAME_MAXLEN);
strlcat(port_cfg->sym_name.symname, BFA_FCS_PORT_SYMBNAME_SEPARATOR,
BFA_SYMNAME_MAXLEN);
@@ -822,7 +822,7 @@ bfa_fcs_fabric_nsymb_init(struct bfa_fcs_fabric_s *fabric)
bfa_ioc_get_adapter_model(&fabric->fcs->bfa->ioc, model);
/* Model name/number */
- strlcpy(port_cfg->node_sym_name.symname, model,
+ strscpy(port_cfg->node_sym_name.symname, model,
BFA_SYMNAME_MAXLEN);
strlcat(port_cfg->node_sym_name.symname,
BFA_FCS_PORT_SYMBNAME_SEPARATOR,
diff --git a/drivers/scsi/bfa/bfa_fcs_lport.c b/drivers/scsi/bfa/bfa_fcs_lport.c
index b12afcc4b189..008afd817087 100644
--- a/drivers/scsi/bfa/bfa_fcs_lport.c
+++ b/drivers/scsi/bfa/bfa_fcs_lport.c
@@ -2642,10 +2642,10 @@ bfa_fcs_fdmi_get_hbaattr(struct bfa_fcs_lport_fdmi_s *fdmi,
bfa_ioc_get_adapter_fw_ver(&port->fcs->bfa->ioc,
hba_attr->fw_version);
- strlcpy(hba_attr->driver_version, (char *)driver_info->version,
+ strscpy(hba_attr->driver_version, (char *)driver_info->version,
sizeof(hba_attr->driver_version));
- strlcpy(hba_attr->os_name, driver_info->host_os_name,
+ strscpy(hba_attr->os_name, driver_info->host_os_name,
sizeof(hba_attr->os_name));
/*
@@ -2663,13 +2663,13 @@ bfa_fcs_fdmi_get_hbaattr(struct bfa_fcs_lport_fdmi_s *fdmi,
bfa_fcs_fdmi_get_portattr(fdmi, &fcs_port_attr);
hba_attr->max_ct_pyld = fcs_port_attr.max_frm_size;
- strlcpy(hba_attr->node_sym_name.symname,
+ strscpy(hba_attr->node_sym_name.symname,
port->port_cfg.node_sym_name.symname, BFA_SYMNAME_MAXLEN);
strcpy(hba_attr->vendor_info, "QLogic");
hba_attr->num_ports =
cpu_to_be32(bfa_ioc_get_nports(&port->fcs->bfa->ioc));
hba_attr->fabric_name = port->fabric->lps->pr_nwwn;
- strlcpy(hba_attr->bios_ver, hba_attr->option_rom_ver, BFA_VERSION_LEN);
+ strscpy(hba_attr->bios_ver, hba_attr->option_rom_ver, BFA_VERSION_LEN);
}
@@ -2736,19 +2736,19 @@ bfa_fcs_fdmi_get_portattr(struct bfa_fcs_lport_fdmi_s *fdmi,
/*
* OS device Name
*/
- strlcpy(port_attr->os_device_name, driver_info->os_device_name,
+ strscpy(port_attr->os_device_name, driver_info->os_device_name,
sizeof(port_attr->os_device_name));
/*
* Host name
*/
- strlcpy(port_attr->host_name, driver_info->host_machine_name,
+ strscpy(port_attr->host_name, driver_info->host_machine_name,
sizeof(port_attr->host_name));
port_attr->node_name = bfa_fcs_lport_get_nwwn(port);
port_attr->port_name = bfa_fcs_lport_get_pwwn(port);
- strlcpy(port_attr->port_sym_name.symname,
+ strscpy(port_attr->port_sym_name.symname,
bfa_fcs_lport_get_psym_name(port).symname, BFA_SYMNAME_MAXLEN);
bfa_fcs_lport_get_attr(port, &lport_attr);
port_attr->port_type = cpu_to_be32(lport_attr.port_type);
@@ -3229,7 +3229,7 @@ bfa_fcs_lport_ms_gmal_response(void *fcsarg, struct bfa_fcxp_s *fcxp,
rsp_str[gmal_entry->len-1] = 0;
/* copy IP Address to fabric */
- strlcpy(bfa_fcs_lport_get_fabric_ipaddr(port),
+ strscpy(bfa_fcs_lport_get_fabric_ipaddr(port),
gmal_entry->ip_addr,
BFA_FCS_FABRIC_IPADDR_SZ);
break;
@@ -4667,7 +4667,7 @@ bfa_fcs_lport_ns_send_rspn_id(void *ns_cbarg, struct bfa_fcxp_s *fcxp_alloced)
* to that of the base port.
*/
- strlcpy(symbl,
+ strscpy(symbl,
(char *)&(bfa_fcs_lport_get_psym_name
(bfa_fcs_get_base_port(port->fcs))),
sizeof(symbl));
@@ -5194,7 +5194,7 @@ bfa_fcs_lport_ns_util_send_rspn_id(void *cbarg, struct bfa_fcxp_s *fcxp_alloced)
* For Vports, we append the vport's port symbolic name
* to that of the base port.
*/
- strlcpy(symbl, (char *)&(bfa_fcs_lport_get_psym_name
+ strscpy(symbl, (char *)&(bfa_fcs_lport_get_psym_name
(bfa_fcs_get_base_port(port->fcs))),
sizeof(symbl));
diff --git a/drivers/scsi/bfa/bfa_ioc.c b/drivers/scsi/bfa/bfa_ioc.c
index 5740302d83ac..e1ed1424fddb 100644
--- a/drivers/scsi/bfa/bfa_ioc.c
+++ b/drivers/scsi/bfa/bfa_ioc.c
@@ -2788,7 +2788,7 @@ void
bfa_ioc_get_adapter_manufacturer(struct bfa_ioc_s *ioc, char *manufacturer)
{
memset((void *)manufacturer, 0, BFA_ADAPTER_MFG_NAME_LEN);
- strlcpy(manufacturer, BFA_MFG_NAME, BFA_ADAPTER_MFG_NAME_LEN);
+ strscpy(manufacturer, BFA_MFG_NAME, BFA_ADAPTER_MFG_NAME_LEN);
}
void
diff --git a/drivers/scsi/bfa/bfa_svc.c b/drivers/scsi/bfa/bfa_svc.c
index 4e3cef02f10f..c9745c0b4eee 100644
--- a/drivers/scsi/bfa/bfa_svc.c
+++ b/drivers/scsi/bfa/bfa_svc.c
@@ -330,7 +330,7 @@ bfa_plog_str(struct bfa_plog_s *plog, enum bfa_plog_mid mid,
lp.eid = event;
lp.log_type = BFA_PL_LOG_TYPE_STRING;
lp.misc = misc;
- strlcpy(lp.log_entry.string_log, log_str,
+ strscpy(lp.log_entry.string_log, log_str,
BFA_PL_STRING_LOG_SZ);
lp.log_entry.string_log[BFA_PL_STRING_LOG_SZ - 1] = '\0';
bfa_plog_add(plog, &lp);
diff --git a/drivers/scsi/bfa/bfad.c b/drivers/scsi/bfa/bfad.c
index 529b73a83d69..62cb7a864fd5 100644
--- a/drivers/scsi/bfa/bfad.c
+++ b/drivers/scsi/bfa/bfad.c
@@ -965,19 +965,19 @@ bfad_start_ops(struct bfad_s *bfad) {
/* Fill the driver_info info to fcs*/
memset(&driver_info, 0, sizeof(driver_info));
- strlcpy(driver_info.version, BFAD_DRIVER_VERSION,
+ strscpy(driver_info.version, BFAD_DRIVER_VERSION,
sizeof(driver_info.version));
if (host_name)
- strlcpy(driver_info.host_machine_name, host_name,
+ strscpy(driver_info.host_machine_name, host_name,
sizeof(driver_info.host_machine_name));
if (os_name)
- strlcpy(driver_info.host_os_name, os_name,
+ strscpy(driver_info.host_os_name, os_name,
sizeof(driver_info.host_os_name));
if (os_patch)
- strlcpy(driver_info.host_os_patch, os_patch,
+ strscpy(driver_info.host_os_patch, os_patch,
sizeof(driver_info.host_os_patch));
- strlcpy(driver_info.os_device_name, bfad->pci_name,
+ strscpy(driver_info.os_device_name, bfad->pci_name,
sizeof(driver_info.os_device_name));
/* FCS driver info init */
diff --git a/drivers/scsi/bfa/bfad_attr.c b/drivers/scsi/bfa/bfad_attr.c
index 5a85401e9e2d..e96e4b6df265 100644
--- a/drivers/scsi/bfa/bfad_attr.c
+++ b/drivers/scsi/bfa/bfad_attr.c
@@ -834,7 +834,7 @@ bfad_im_symbolic_name_show(struct device *dev, struct device_attribute *attr,
char symname[BFA_SYMNAME_MAXLEN];
bfa_fcs_lport_get_attr(&bfad->bfa_fcs.fabric.bport, &port_attr);
- strlcpy(symname, port_attr.port_cfg.sym_name.symname,
+ strscpy(symname, port_attr.port_cfg.sym_name.symname,
BFA_SYMNAME_MAXLEN);
return sysfs_emit(buf, "%s\n", symname);
}
diff --git a/drivers/scsi/bfa/bfad_bsg.c b/drivers/scsi/bfa/bfad_bsg.c
index 79d4f7ee5bcb..520f9152f3bf 100644
--- a/drivers/scsi/bfa/bfad_bsg.c
+++ b/drivers/scsi/bfa/bfad_bsg.c
@@ -119,7 +119,7 @@ bfad_iocmd_ioc_get_attr(struct bfad_s *bfad, void *cmd)
/* fill in driver attr info */
strcpy(iocmd->ioc_attr.driver_attr.driver, BFAD_DRIVER_NAME);
- strlcpy(iocmd->ioc_attr.driver_attr.driver_ver,
+ strscpy(iocmd->ioc_attr.driver_attr.driver_ver,
BFAD_DRIVER_VERSION, BFA_VERSION_LEN);
strcpy(iocmd->ioc_attr.driver_attr.fw_ver,
iocmd->ioc_attr.adapter_attr.fw_ver);
@@ -307,7 +307,7 @@ bfad_iocmd_port_get_attr(struct bfad_s *bfad, void *cmd)
iocmd->attr.port_type = port_attr.port_type;
iocmd->attr.loopback = port_attr.loopback;
iocmd->attr.authfail = port_attr.authfail;
- strlcpy(iocmd->attr.port_symname.symname,
+ strscpy(iocmd->attr.port_symname.symname,
port_attr.port_cfg.sym_name.symname,
sizeof(iocmd->attr.port_symname.symname));
diff --git a/drivers/scsi/bfa/bfad_im.c b/drivers/scsi/bfa/bfad_im.c
index c335f7a188d2..a9d3d8562d3c 100644
--- a/drivers/scsi/bfa/bfad_im.c
+++ b/drivers/scsi/bfa/bfad_im.c
@@ -1046,7 +1046,7 @@ bfad_fc_host_init(struct bfad_im_port_s *im_port)
/* For fibre channel services type 0x20 */
fc_host_supported_fc4s(host)[7] = 1;
- strlcpy(symname, bfad->bfa_fcs.fabric.bport.port_cfg.sym_name.symname,
+ strscpy(symname, bfad->bfa_fcs.fabric.bport.port_cfg.sym_name.symname,
BFA_SYMNAME_MAXLEN);
sprintf(fc_host_symbolic_name(host), "%s", symname);
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] scsi: bfa: Replace all non-returning strlcpy with strscpy
2023-05-16 1:33 [PATCH] scsi: bfa: Replace all non-returning strlcpy with strscpy Azeem Shaikh
@ 2023-05-16 2:02 ` Azeem Shaikh
2023-05-16 18:15 ` Kees Cook
2023-05-17 1:38 ` Martin K. Petersen
2023-05-22 22:46 ` Martin K. Petersen
2 siblings, 1 reply; 5+ messages in thread
From: Azeem Shaikh @ 2023-05-16 2:02 UTC (permalink / raw)
To: Anil Gurumurthy, Sudarsana Kalluru
Cc: linux-scsi, linux-kernel, James E.J. Bottomley,
Martin K. Petersen, linux-hardening
cc linux-hardening@vger.kernel.org
On Mon, May 15, 2023 at 9:35 PM Azeem Shaikh <azeemshaikh38@gmail.com> wrote:
>
> 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/scsi/bfa/bfa_fcbuild.c | 4 ++--
> drivers/scsi/bfa/bfa_fcs.c | 4 ++--
> drivers/scsi/bfa/bfa_fcs_lport.c | 20 ++++++++++----------
> drivers/scsi/bfa/bfa_ioc.c | 2 +-
> drivers/scsi/bfa/bfa_svc.c | 2 +-
> drivers/scsi/bfa/bfad.c | 10 +++++-----
> drivers/scsi/bfa/bfad_attr.c | 2 +-
> drivers/scsi/bfa/bfad_bsg.c | 4 ++--
> drivers/scsi/bfa/bfad_im.c | 2 +-
> 9 files changed, 25 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/scsi/bfa/bfa_fcbuild.c b/drivers/scsi/bfa/bfa_fcbuild.c
> index df18d9d2af53..773c84af784c 100644
> --- a/drivers/scsi/bfa/bfa_fcbuild.c
> +++ b/drivers/scsi/bfa/bfa_fcbuild.c
> @@ -1134,7 +1134,7 @@ fc_rspnid_build(struct fchs_s *fchs, void *pyld, u32 s_id, u16 ox_id,
> memset(rspnid, 0, sizeof(struct fcgs_rspnid_req_s));
>
> rspnid->dap = s_id;
> - strlcpy(rspnid->spn, name, sizeof(rspnid->spn));
> + strscpy(rspnid->spn, name, sizeof(rspnid->spn));
> rspnid->spn_len = (u8) strlen(rspnid->spn);
>
> return sizeof(struct fcgs_rspnid_req_s) + sizeof(struct ct_hdr_s);
> @@ -1155,7 +1155,7 @@ fc_rsnn_nn_build(struct fchs_s *fchs, void *pyld, u32 s_id,
> memset(rsnn_nn, 0, sizeof(struct fcgs_rsnn_nn_req_s));
>
> rsnn_nn->node_name = node_name;
> - strlcpy(rsnn_nn->snn, name, sizeof(rsnn_nn->snn));
> + strscpy(rsnn_nn->snn, name, sizeof(rsnn_nn->snn));
> rsnn_nn->snn_len = (u8) strlen(rsnn_nn->snn);
>
> return sizeof(struct fcgs_rsnn_nn_req_s) + sizeof(struct ct_hdr_s);
> diff --git a/drivers/scsi/bfa/bfa_fcs.c b/drivers/scsi/bfa/bfa_fcs.c
> index d2d396ca0e9a..5023c0ab4277 100644
> --- a/drivers/scsi/bfa/bfa_fcs.c
> +++ b/drivers/scsi/bfa/bfa_fcs.c
> @@ -761,7 +761,7 @@ bfa_fcs_fabric_psymb_init(struct bfa_fcs_fabric_s *fabric)
> bfa_ioc_get_adapter_model(&fabric->fcs->bfa->ioc, model);
>
> /* Model name/number */
> - strlcpy(port_cfg->sym_name.symname, model,
> + strscpy(port_cfg->sym_name.symname, model,
> BFA_SYMNAME_MAXLEN);
> strlcat(port_cfg->sym_name.symname, BFA_FCS_PORT_SYMBNAME_SEPARATOR,
> BFA_SYMNAME_MAXLEN);
> @@ -822,7 +822,7 @@ bfa_fcs_fabric_nsymb_init(struct bfa_fcs_fabric_s *fabric)
> bfa_ioc_get_adapter_model(&fabric->fcs->bfa->ioc, model);
>
> /* Model name/number */
> - strlcpy(port_cfg->node_sym_name.symname, model,
> + strscpy(port_cfg->node_sym_name.symname, model,
> BFA_SYMNAME_MAXLEN);
> strlcat(port_cfg->node_sym_name.symname,
> BFA_FCS_PORT_SYMBNAME_SEPARATOR,
> diff --git a/drivers/scsi/bfa/bfa_fcs_lport.c b/drivers/scsi/bfa/bfa_fcs_lport.c
> index b12afcc4b189..008afd817087 100644
> --- a/drivers/scsi/bfa/bfa_fcs_lport.c
> +++ b/drivers/scsi/bfa/bfa_fcs_lport.c
> @@ -2642,10 +2642,10 @@ bfa_fcs_fdmi_get_hbaattr(struct bfa_fcs_lport_fdmi_s *fdmi,
> bfa_ioc_get_adapter_fw_ver(&port->fcs->bfa->ioc,
> hba_attr->fw_version);
>
> - strlcpy(hba_attr->driver_version, (char *)driver_info->version,
> + strscpy(hba_attr->driver_version, (char *)driver_info->version,
> sizeof(hba_attr->driver_version));
>
> - strlcpy(hba_attr->os_name, driver_info->host_os_name,
> + strscpy(hba_attr->os_name, driver_info->host_os_name,
> sizeof(hba_attr->os_name));
>
> /*
> @@ -2663,13 +2663,13 @@ bfa_fcs_fdmi_get_hbaattr(struct bfa_fcs_lport_fdmi_s *fdmi,
> bfa_fcs_fdmi_get_portattr(fdmi, &fcs_port_attr);
> hba_attr->max_ct_pyld = fcs_port_attr.max_frm_size;
>
> - strlcpy(hba_attr->node_sym_name.symname,
> + strscpy(hba_attr->node_sym_name.symname,
> port->port_cfg.node_sym_name.symname, BFA_SYMNAME_MAXLEN);
> strcpy(hba_attr->vendor_info, "QLogic");
> hba_attr->num_ports =
> cpu_to_be32(bfa_ioc_get_nports(&port->fcs->bfa->ioc));
> hba_attr->fabric_name = port->fabric->lps->pr_nwwn;
> - strlcpy(hba_attr->bios_ver, hba_attr->option_rom_ver, BFA_VERSION_LEN);
> + strscpy(hba_attr->bios_ver, hba_attr->option_rom_ver, BFA_VERSION_LEN);
>
> }
>
> @@ -2736,19 +2736,19 @@ bfa_fcs_fdmi_get_portattr(struct bfa_fcs_lport_fdmi_s *fdmi,
> /*
> * OS device Name
> */
> - strlcpy(port_attr->os_device_name, driver_info->os_device_name,
> + strscpy(port_attr->os_device_name, driver_info->os_device_name,
> sizeof(port_attr->os_device_name));
>
> /*
> * Host name
> */
> - strlcpy(port_attr->host_name, driver_info->host_machine_name,
> + strscpy(port_attr->host_name, driver_info->host_machine_name,
> sizeof(port_attr->host_name));
>
> port_attr->node_name = bfa_fcs_lport_get_nwwn(port);
> port_attr->port_name = bfa_fcs_lport_get_pwwn(port);
>
> - strlcpy(port_attr->port_sym_name.symname,
> + strscpy(port_attr->port_sym_name.symname,
> bfa_fcs_lport_get_psym_name(port).symname, BFA_SYMNAME_MAXLEN);
> bfa_fcs_lport_get_attr(port, &lport_attr);
> port_attr->port_type = cpu_to_be32(lport_attr.port_type);
> @@ -3229,7 +3229,7 @@ bfa_fcs_lport_ms_gmal_response(void *fcsarg, struct bfa_fcxp_s *fcxp,
> rsp_str[gmal_entry->len-1] = 0;
>
> /* copy IP Address to fabric */
> - strlcpy(bfa_fcs_lport_get_fabric_ipaddr(port),
> + strscpy(bfa_fcs_lport_get_fabric_ipaddr(port),
> gmal_entry->ip_addr,
> BFA_FCS_FABRIC_IPADDR_SZ);
> break;
> @@ -4667,7 +4667,7 @@ bfa_fcs_lport_ns_send_rspn_id(void *ns_cbarg, struct bfa_fcxp_s *fcxp_alloced)
> * to that of the base port.
> */
>
> - strlcpy(symbl,
> + strscpy(symbl,
> (char *)&(bfa_fcs_lport_get_psym_name
> (bfa_fcs_get_base_port(port->fcs))),
> sizeof(symbl));
> @@ -5194,7 +5194,7 @@ bfa_fcs_lport_ns_util_send_rspn_id(void *cbarg, struct bfa_fcxp_s *fcxp_alloced)
> * For Vports, we append the vport's port symbolic name
> * to that of the base port.
> */
> - strlcpy(symbl, (char *)&(bfa_fcs_lport_get_psym_name
> + strscpy(symbl, (char *)&(bfa_fcs_lport_get_psym_name
> (bfa_fcs_get_base_port(port->fcs))),
> sizeof(symbl));
>
> diff --git a/drivers/scsi/bfa/bfa_ioc.c b/drivers/scsi/bfa/bfa_ioc.c
> index 5740302d83ac..e1ed1424fddb 100644
> --- a/drivers/scsi/bfa/bfa_ioc.c
> +++ b/drivers/scsi/bfa/bfa_ioc.c
> @@ -2788,7 +2788,7 @@ void
> bfa_ioc_get_adapter_manufacturer(struct bfa_ioc_s *ioc, char *manufacturer)
> {
> memset((void *)manufacturer, 0, BFA_ADAPTER_MFG_NAME_LEN);
> - strlcpy(manufacturer, BFA_MFG_NAME, BFA_ADAPTER_MFG_NAME_LEN);
> + strscpy(manufacturer, BFA_MFG_NAME, BFA_ADAPTER_MFG_NAME_LEN);
> }
>
> void
> diff --git a/drivers/scsi/bfa/bfa_svc.c b/drivers/scsi/bfa/bfa_svc.c
> index 4e3cef02f10f..c9745c0b4eee 100644
> --- a/drivers/scsi/bfa/bfa_svc.c
> +++ b/drivers/scsi/bfa/bfa_svc.c
> @@ -330,7 +330,7 @@ bfa_plog_str(struct bfa_plog_s *plog, enum bfa_plog_mid mid,
> lp.eid = event;
> lp.log_type = BFA_PL_LOG_TYPE_STRING;
> lp.misc = misc;
> - strlcpy(lp.log_entry.string_log, log_str,
> + strscpy(lp.log_entry.string_log, log_str,
> BFA_PL_STRING_LOG_SZ);
> lp.log_entry.string_log[BFA_PL_STRING_LOG_SZ - 1] = '\0';
> bfa_plog_add(plog, &lp);
> diff --git a/drivers/scsi/bfa/bfad.c b/drivers/scsi/bfa/bfad.c
> index 529b73a83d69..62cb7a864fd5 100644
> --- a/drivers/scsi/bfa/bfad.c
> +++ b/drivers/scsi/bfa/bfad.c
> @@ -965,19 +965,19 @@ bfad_start_ops(struct bfad_s *bfad) {
>
> /* Fill the driver_info info to fcs*/
> memset(&driver_info, 0, sizeof(driver_info));
> - strlcpy(driver_info.version, BFAD_DRIVER_VERSION,
> + strscpy(driver_info.version, BFAD_DRIVER_VERSION,
> sizeof(driver_info.version));
> if (host_name)
> - strlcpy(driver_info.host_machine_name, host_name,
> + strscpy(driver_info.host_machine_name, host_name,
> sizeof(driver_info.host_machine_name));
> if (os_name)
> - strlcpy(driver_info.host_os_name, os_name,
> + strscpy(driver_info.host_os_name, os_name,
> sizeof(driver_info.host_os_name));
> if (os_patch)
> - strlcpy(driver_info.host_os_patch, os_patch,
> + strscpy(driver_info.host_os_patch, os_patch,
> sizeof(driver_info.host_os_patch));
>
> - strlcpy(driver_info.os_device_name, bfad->pci_name,
> + strscpy(driver_info.os_device_name, bfad->pci_name,
> sizeof(driver_info.os_device_name));
>
> /* FCS driver info init */
> diff --git a/drivers/scsi/bfa/bfad_attr.c b/drivers/scsi/bfa/bfad_attr.c
> index 5a85401e9e2d..e96e4b6df265 100644
> --- a/drivers/scsi/bfa/bfad_attr.c
> +++ b/drivers/scsi/bfa/bfad_attr.c
> @@ -834,7 +834,7 @@ bfad_im_symbolic_name_show(struct device *dev, struct device_attribute *attr,
> char symname[BFA_SYMNAME_MAXLEN];
>
> bfa_fcs_lport_get_attr(&bfad->bfa_fcs.fabric.bport, &port_attr);
> - strlcpy(symname, port_attr.port_cfg.sym_name.symname,
> + strscpy(symname, port_attr.port_cfg.sym_name.symname,
> BFA_SYMNAME_MAXLEN);
> return sysfs_emit(buf, "%s\n", symname);
> }
> diff --git a/drivers/scsi/bfa/bfad_bsg.c b/drivers/scsi/bfa/bfad_bsg.c
> index 79d4f7ee5bcb..520f9152f3bf 100644
> --- a/drivers/scsi/bfa/bfad_bsg.c
> +++ b/drivers/scsi/bfa/bfad_bsg.c
> @@ -119,7 +119,7 @@ bfad_iocmd_ioc_get_attr(struct bfad_s *bfad, void *cmd)
>
> /* fill in driver attr info */
> strcpy(iocmd->ioc_attr.driver_attr.driver, BFAD_DRIVER_NAME);
> - strlcpy(iocmd->ioc_attr.driver_attr.driver_ver,
> + strscpy(iocmd->ioc_attr.driver_attr.driver_ver,
> BFAD_DRIVER_VERSION, BFA_VERSION_LEN);
> strcpy(iocmd->ioc_attr.driver_attr.fw_ver,
> iocmd->ioc_attr.adapter_attr.fw_ver);
> @@ -307,7 +307,7 @@ bfad_iocmd_port_get_attr(struct bfad_s *bfad, void *cmd)
> iocmd->attr.port_type = port_attr.port_type;
> iocmd->attr.loopback = port_attr.loopback;
> iocmd->attr.authfail = port_attr.authfail;
> - strlcpy(iocmd->attr.port_symname.symname,
> + strscpy(iocmd->attr.port_symname.symname,
> port_attr.port_cfg.sym_name.symname,
> sizeof(iocmd->attr.port_symname.symname));
>
> diff --git a/drivers/scsi/bfa/bfad_im.c b/drivers/scsi/bfa/bfad_im.c
> index c335f7a188d2..a9d3d8562d3c 100644
> --- a/drivers/scsi/bfa/bfad_im.c
> +++ b/drivers/scsi/bfa/bfad_im.c
> @@ -1046,7 +1046,7 @@ bfad_fc_host_init(struct bfad_im_port_s *im_port)
> /* For fibre channel services type 0x20 */
> fc_host_supported_fc4s(host)[7] = 1;
>
> - strlcpy(symname, bfad->bfa_fcs.fabric.bport.port_cfg.sym_name.symname,
> + strscpy(symname, bfad->bfa_fcs.fabric.bport.port_cfg.sym_name.symname,
> BFA_SYMNAME_MAXLEN);
> sprintf(fc_host_symbolic_name(host), "%s", symname);
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] scsi: bfa: Replace all non-returning strlcpy with strscpy
2023-05-16 2:02 ` Azeem Shaikh
@ 2023-05-16 18:15 ` Kees Cook
0 siblings, 0 replies; 5+ messages in thread
From: Kees Cook @ 2023-05-16 18:15 UTC (permalink / raw)
To: Azeem Shaikh
Cc: Anil Gurumurthy, Sudarsana Kalluru, linux-scsi, linux-kernel,
James E.J. Bottomley, Martin K. Petersen, linux-hardening
> On Mon, May 15, 2023 at 9:35 PM Azeem Shaikh <azeemshaikh38@gmail.com> wrote:
> >
> > 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>
Reviewed-by: Kees Cook <keescook@chromium.org>
--
Kees Cook
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] scsi: bfa: Replace all non-returning strlcpy with strscpy
2023-05-16 1:33 [PATCH] scsi: bfa: Replace all non-returning strlcpy with strscpy Azeem Shaikh
2023-05-16 2:02 ` Azeem Shaikh
@ 2023-05-17 1:38 ` Martin K. Petersen
2023-05-22 22:46 ` Martin K. Petersen
2 siblings, 0 replies; 5+ messages in thread
From: Martin K. Petersen @ 2023-05-17 1:38 UTC (permalink / raw)
To: Azeem Shaikh
Cc: Anil Gurumurthy, Sudarsana Kalluru, azeems, linux-scsi,
linux-kernel, James E.J. Bottomley, Martin K. Petersen
Azeem,
> 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!
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] scsi: bfa: Replace all non-returning strlcpy with strscpy
2023-05-16 1:33 [PATCH] scsi: bfa: Replace all non-returning strlcpy with strscpy Azeem Shaikh
2023-05-16 2:02 ` Azeem Shaikh
2023-05-17 1:38 ` Martin K. Petersen
@ 2023-05-22 22:46 ` Martin K. Petersen
2 siblings, 0 replies; 5+ messages in thread
From: Martin K. Petersen @ 2023-05-22 22:46 UTC (permalink / raw)
To: Anil Gurumurthy, Sudarsana Kalluru, Azeem Shaikh
Cc: Martin K . Petersen, azeems, linux-scsi, linux-kernel,
James E.J. Bottomley
On Tue, 16 May 2023 01:33:45 +0000, Azeem Shaikh wrote:
> 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: bfa: Replace all non-returning strlcpy with strscpy
https://git.kernel.org/mkp/scsi/c/973464fded69
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-05-22 22:46 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-05-16 1:33 [PATCH] scsi: bfa: Replace all non-returning strlcpy with strscpy Azeem Shaikh
2023-05-16 2:02 ` Azeem Shaikh
2023-05-16 18:15 ` Kees Cook
2023-05-17 1:38 ` Martin K. Petersen
2023-05-22 22:46 ` Martin K. Petersen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox