[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <20230927-strncpy-drivers-mmc-host-vub300-c-v1-1-77426f62eef4@google.com>
Date: Wed, 27 Sep 2023 06:41:37 +0000
From: Justin Stitt <justinstitt@...gle.com>
To: Ulf Hansson <ulf.hansson@...aro.org>
Cc: linux-mmc@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-hardening@...r.kernel.org,
Justin Stitt <justinstitt@...gle.com>
Subject: [PATCH] mmc: vub300: 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 `vub300->vub_name` to be NUL-terminated based on its uses with
format strings:
| dev_info(&vub300->udev->dev, "using %s for SDIO offload processing\n",
| vub300->vub_name);
NUL-padding is not needed. We can see cleaning out vub_name simply
consists of:
| vub300->vub_name[0] = 0;
Considering the above, for all 11 cases a suitable replacement is
`strscpy` [2] due to the fact that it guarantees NUL-termination on the
destination buffer without unnecessarily NUL-padding.
To be clear, there is no existing bug in the current implementation as
the string literals are all small enough as to not cause a buffer
overread. Nonetheless, this gets us 11 steps closer to removing strncpy
uses.
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@...r.kernel.org
Signed-off-by: Justin Stitt <justinstitt@...gle.com>
---
Note: build-tested only.
---
drivers/mmc/host/vub300.c | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/drivers/mmc/host/vub300.c b/drivers/mmc/host/vub300.c
index 9ec593d52f0f..de3f443f5fdc 100644
--- a/drivers/mmc/host/vub300.c
+++ b/drivers/mmc/host/vub300.c
@@ -512,7 +512,7 @@ static void new_system_port_status(struct vub300_mmc_host *vub300)
vub300->card_present = 1;
vub300->bus_width = 0;
if (disable_offload_processing)
- strncpy(vub300->vub_name, "EMPTY Processing Disabled",
+ strscpy(vub300->vub_name, "EMPTY Processing Disabled",
sizeof(vub300->vub_name));
else
vub300->vub_name[0] = 0;
@@ -1216,7 +1216,7 @@ static void __download_offload_pseudocode(struct vub300_mmc_host *vub300,
dev_err(&vub300->udev->dev,
"corrupt offload pseudocode in firmware %s\n",
vub300->vub_name);
- strncpy(vub300->vub_name, "corrupt offload pseudocode",
+ strscpy(vub300->vub_name, "corrupt offload pseudocode",
sizeof(vub300->vub_name));
return;
}
@@ -1250,7 +1250,7 @@ static void __download_offload_pseudocode(struct vub300_mmc_host *vub300,
"not enough memory for xfer buffer to send"
" INTERRUPT_PSEUDOCODE for %s %s\n", fw->data,
vub300->vub_name);
- strncpy(vub300->vub_name,
+ strscpy(vub300->vub_name,
"SDIO interrupt pseudocode download failed",
sizeof(vub300->vub_name));
return;
@@ -1259,7 +1259,7 @@ static void __download_offload_pseudocode(struct vub300_mmc_host *vub300,
dev_err(&vub300->udev->dev,
"corrupt interrupt pseudocode in firmware %s %s\n",
fw->data, vub300->vub_name);
- strncpy(vub300->vub_name, "corrupt interrupt pseudocode",
+ strscpy(vub300->vub_name, "corrupt interrupt pseudocode",
sizeof(vub300->vub_name));
return;
}
@@ -1293,7 +1293,7 @@ static void __download_offload_pseudocode(struct vub300_mmc_host *vub300,
"not enough memory for xfer buffer to send"
" TRANSFER_PSEUDOCODE for %s %s\n", fw->data,
vub300->vub_name);
- strncpy(vub300->vub_name,
+ strscpy(vub300->vub_name,
"SDIO transfer pseudocode download failed",
sizeof(vub300->vub_name));
return;
@@ -1302,7 +1302,7 @@ static void __download_offload_pseudocode(struct vub300_mmc_host *vub300,
dev_err(&vub300->udev->dev,
"corrupt transfer pseudocode in firmware %s %s\n",
fw->data, vub300->vub_name);
- strncpy(vub300->vub_name, "corrupt transfer pseudocode",
+ strscpy(vub300->vub_name, "corrupt transfer pseudocode",
sizeof(vub300->vub_name));
return;
}
@@ -1336,13 +1336,13 @@ static void __download_offload_pseudocode(struct vub300_mmc_host *vub300,
dev_err(&vub300->udev->dev,
"corrupt dynamic registers in firmware %s\n",
vub300->vub_name);
- strncpy(vub300->vub_name, "corrupt dynamic registers",
+ strscpy(vub300->vub_name, "corrupt dynamic registers",
sizeof(vub300->vub_name));
return;
}
copy_error_message:
- strncpy(vub300->vub_name, "SDIO pseudocode download failed",
+ strscpy(vub300->vub_name, "SDIO pseudocode download failed",
sizeof(vub300->vub_name));
}
@@ -1370,11 +1370,11 @@ static void download_offload_pseudocode(struct vub300_mmc_host *vub300)
vub300->vub_name);
retval = request_firmware(&fw, vub300->vub_name, &card->dev);
if (retval < 0) {
- strncpy(vub300->vub_name, "vub_default.bin",
+ strscpy(vub300->vub_name, "vub_default.bin",
sizeof(vub300->vub_name));
retval = request_firmware(&fw, vub300->vub_name, &card->dev);
if (retval < 0) {
- strncpy(vub300->vub_name,
+ strscpy(vub300->vub_name,
"no SDIO offload firmware found",
sizeof(vub300->vub_name));
} else {
@@ -1758,7 +1758,7 @@ static void vub300_cmndwork_thread(struct work_struct *work)
* has been already downloaded to the VUB300 chip
*/
} else if (0 == vub300->mmc->card->sdio_funcs) {
- strncpy(vub300->vub_name, "SD memory device",
+ strscpy(vub300->vub_name, "SD memory device",
sizeof(vub300->vub_name));
} else {
download_offload_pseudocode(vub300);
---
base-commit: 6465e260f48790807eef06b583b38ca9789b6072
change-id: 20230927-strncpy-drivers-mmc-host-vub300-c-b7b39f82e584
Best regards,
--
Justin Stitt <justinstitt@...gle.com>
Powered by blists - more mailing lists