Re: [PATCH] staging: greybus: fix string termination in firmware sample

[Kurmapu Sai Devansh Prasad]

On Mon, Nov 2, 2026 at 8:13 PM Dan Carpenter <error27@xxxxxxxxx> wrote:
>
> On Mon, Nov 02, 2026 at 07:44:09PM +0530, Kurmapu Sai Devansh Prasad wrote:
> > >From 9114eb9d60f2c4a52f05e3a33b0d6b8ca4856cac Mon Sep 17 00:00:00 2001
> > From: Kurmapu Sai Devansh Prasad <btech10487.26@xxxxxxxxxxxxxx>
> > Date: Mon, 2 Nov 2026 19:11:09 +0530
> > Subject: [PATCH] staging: greybus: fix string termination in firmware sample
> >
> > The firmware_tag string copied via strncpy() is not guaranteed to be
> > NUL-terminated if the source string is exactly the size of the destination
> > buffer. This can lead to a buffer over-read.
> >
> >  Since alternatives like strscpy_pad cannot be used in user space
> > sample code, we can make it  safer by limiting the copy size to
> > MAX_SIZE - 1 and explicitly assigning the last byte to be a NUL
> > terminator. This ensures the string is always
> > safely terminated before being passed to the ioctl.
>
> In user space, using strncpy() is fine.
>
> >
> > Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings
> >
> > This is my very first patch to the Linux kernel! I've been learning the
> > process and would deeply appreciate any feedback from the maintainers.
>
> Put this sort of unrelated conversational stuff under the --- cut off
>
> >
> > Signed-off-by: Kurmapu Sai Devansh Prasad <btech10487.26@xxxxxxxxxxxxxx>
> > ---
>   ^^^
> here.  So that it doesn't get saved into the kernel git log.
>
> The patch is pretty badly corrupted and doesn't apply.  Read the first
> two paragraph of Documentation/process/email-clients.rst
>
> regards,
> dan carpenter
>
From 8ac1b813a52037a360d58e8e345b661193258b61 Mon Sep 17 00:00:00 2001
From: Kurmapu Sai Devansh Prasad <btech10487.26@xxxxxxxxxxxxxx>
Date: Mon, 2 Nov 2026 19:11:09 +0530
Subject: [PATCH v2] staging: greybus: fix string termination in firmware
 sample

The firmware_tag string copied via strncpy() is not guaranteed to be
NUL-terminated if the source string is exactly the size of the destination
buffer. This can lead to a buffer over-read.

Since alternatives like strscpy_pad cannot be used in user space sample code, we can make it safer by limiting
the copy size to MAX_SIZE - 1 and explicitly by assigning the last byte to be a NUL terminator.

Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings

Signed-off-by: Kurmapu Sai Devansh Prasad <btech10487.26@xxxxxxxxxxxxxx>
---
Hello  Dan, thanks for the advice. I've attached the patch this time to avoid the Gmail formatting issues.
 I also removed the extra commentary from the commit message.
---
 .../greybus/Documentation/firmware/firmware.c        | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/greybus/Documentation/firmware/firmware.c b/drivers/staging/greybus/Documentation/firmware/firmware.c
index 3b35ef6d4adb..a6a0988ad6fe 100644
--- a/drivers/staging/greybus/Documentation/firmware/firmware.c
+++ b/drivers/staging/greybus/Documentation/firmware/firmware.c
@@ -64,7 +64,9 @@ static int update_intf_firmware(int fd)
 	intf_load.minor = 0;
 
 	strncpy((char *)&intf_load.firmware_tag, firmware_tag,
-		GB_FIRMWARE_U_TAG_MAX_SIZE);
+		GB_FIRMWARE_U_TAG_MAX_SIZE - 1);
+	((char *)&intf_load.firmware_tag)[GB_FIRMWARE_U_TAG_MAX_SIZE - 1] =
+		'\0';
 
 	ret = ioctl(fd, FW_MGMT_IOC_INTF_LOAD_AND_VALIDATE, &intf_load);
 	if (ret < 0) {
@@ -102,7 +104,9 @@ static int update_backend_firmware(int fd)
 	printf("Getting Backend Firmware Version\n");
 
 	strncpy((char *)&backend_fw_info.firmware_tag, firmware_tag,
-		GB_FIRMWARE_U_TAG_MAX_SIZE);
+		GB_FIRMWARE_U_TAG_MAX_SIZE - 1);
+	((char *)&backend_fw_info.firmware_tag)[GB_FIRMWARE_U_TAG_MAX_SIZE - 1] =
+		'\0';
 
 retry_fw_version:
 	ret = ioctl(fd, FW_MGMT_IOC_GET_BACKEND_FW, &backend_fw_info);
@@ -130,7 +134,9 @@ static int update_backend_firmware(int fd)
 	printf("Updating Backend Firmware\n");
 
 	strncpy((char *)&backend_update.firmware_tag, firmware_tag,
-		GB_FIRMWARE_U_TAG_MAX_SIZE);
+		GB_FIRMWARE_U_TAG_MAX_SIZE - 1);
+	((char *)&backend_update.firmware_tag)[GB_FIRMWARE_U_TAG_MAX_SIZE - 1] =
+		'\0';
 
 retry_fw_update:
 	backend_update.status = 0;
-- 
2.56.0