summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJustin Stitt <justinstitt@google.com>2023-10-30 19:04:33 +0000
committerMartin K. Petersen <martin.petersen@oracle.com>2023-12-05 21:01:52 -0500
commita9baa16b4fc11d07953b65298b05d5ee707f093a (patch)
treead47f959a0785099591c7eaa1e953ae36cc094e2
parent0d224b1088afef95f7069b7ca907cb199a5802ff (diff)
scsi: ibmvfc: Replace deprecated strncpy() with strscpy()
strncpy() is deprecated for use on NUL-terminated destination strings [1] and as such we should prefer more robust and less ambiguous string interfaces. We expect these fields to be NUL-terminated as the property names from which they are derived are also NUL-terminated. Moreover, NUL-padding is not required as our destination buffers are already NUL-allocated and any future NUL-byte assignments are redundant (like the ones that strncpy() does). ibmvfc_probe() -> | struct ibmvfc_host *vhost; | struct Scsi_Host *shost; ... | shost = scsi_host_alloc(&driver_template, sizeof(*vhost)); ... **side note: is this a bug? Looks like a type to me ^^^^^** ... | vhost = shost_priv(shost); ... where shost_priv() is: | static inline void *shost_priv(struct Scsi_Host *shost) | { | return (void *)shost->hostdata; | } .. and: scsi_host_alloc() -> | shost = kzalloc(sizeof(struct Scsi_Host) + privsize, GFP_KERNEL); And for login_info->..., NUL-padding is also not required as it is explicitly memset to 0: | memset(login_info, 0, sizeof(*login_info)); Considering the above, a suitable replacement is strscpy() [2] due to the fact that it guarantees NUL-termination on the destination buffer without unnecessarily NUL-padding. Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings [1] Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html [2] Link: https://github.com/KSPP/linux/issues/90 Cc: <linux-hardening@vger.kernel.org> Signed-off-by: Justin Stitt <justinstitt@google.com> Link: https://lore.kernel.org/r/20231030-strncpy-drivers-scsi-ibmvscsi-ibmvfc-c-v1-1-5a4909688435@google.com Reviewed-by: Kees Cook <keescook@chromium.org> Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
-rw-r--r--drivers/scsi/ibmvscsi/ibmvfc.c12
1 files changed, 7 insertions, 5 deletions
diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
index 92c440f2e3a7..46d0b3a0e12f 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc.c
@@ -1464,7 +1464,7 @@ static void ibmvfc_gather_partition_info(struct ibmvfc_host *vhost)
name = of_get_property(rootdn, "ibm,partition-name", NULL);
if (name)
- strncpy(vhost->partition_name, name, sizeof(vhost->partition_name));
+ strscpy(vhost->partition_name, name, sizeof(vhost->partition_name));
num = of_get_property(rootdn, "ibm,partition-no", NULL);
if (num)
vhost->partition_number = *num;
@@ -1513,13 +1513,15 @@ static void ibmvfc_set_login_info(struct ibmvfc_host *vhost)
login_info->async.va = cpu_to_be64(vhost->async_crq.msg_token);
login_info->async.len = cpu_to_be32(async_crq->size *
sizeof(*async_crq->msgs.async));
- strncpy(login_info->partition_name, vhost->partition_name, IBMVFC_MAX_NAME);
- strncpy(login_info->device_name,
- dev_name(&vhost->host->shost_gendev), IBMVFC_MAX_NAME);
+ strscpy(login_info->partition_name, vhost->partition_name,
+ sizeof(login_info->partition_name));
+
+ strscpy(login_info->device_name,
+ dev_name(&vhost->host->shost_gendev), sizeof(login_info->device_name));
location = of_get_property(of_node, "ibm,loc-code", NULL);
location = location ? location : dev_name(vhost->dev);
- strncpy(login_info->drc_name, location, IBMVFC_MAX_NAME);
+ strscpy(login_info->drc_name, location, sizeof(login_info->drc_name));
}
/**