diff options
author | Justin Stitt <justinstitt@google.com> | 2023-10-30 19:04:33 +0000 |
---|---|---|
committer | Martin K. Petersen <martin.petersen@oracle.com> | 2023-12-05 21:01:52 -0500 |
commit | a9baa16b4fc11d07953b65298b05d5ee707f093a (patch) | |
tree | ad47f959a0785099591c7eaa1e953ae36cc094e2 | |
parent | 0d224b1088afef95f7069b7ca907cb199a5802ff (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.c | 12 |
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)); } /** |