diff mbox series

mmc: vub300: replace deprecated strncpy with strscpy

Message ID 20230927-strncpy-drivers-mmc-host-vub300-c-v1-1-77426f62eef4@google.com (mailing list archive)
State Mainlined
Commit 4b9b94766534ea23bffc8606b73fb18501b49a67
Headers show
Series mmc: vub300: replace deprecated strncpy with strscpy | expand

Commit Message

Justin Stitt Sept. 27, 2023, 6:41 a.m. UTC
`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@vger.kernel.org
Signed-off-by: Justin Stitt <justinstitt@google.com>
---
Note: build-tested only.
---
 drivers/mmc/host/vub300.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)


---
base-commit: 6465e260f48790807eef06b583b38ca9789b6072
change-id: 20230927-strncpy-drivers-mmc-host-vub300-c-b7b39f82e584

Best regards,
--
Justin Stitt <justinstitt@google.com>

Comments

Ulf Hansson Sept. 27, 2023, 10:40 a.m. UTC | #1
On Wed, 27 Sept 2023 at 08:41, Justin Stitt <justinstitt@google.com> wrote:
>
> `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@vger.kernel.org
> Signed-off-by: Justin Stitt <justinstitt@google.com>

Applied for next, thanks!

Kind regards
Uffe


> ---
> 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@google.com>
>
diff mbox series

Patch

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);