[PATCH] tools/leds/uledmon: force NUL-termination of device name

[PATCH] tools/leds/uledmon: force NUL-termination of device name

[Gu Bin]

strncpy() does not NUL-terminate when the source is at least as long
as the bound. uleds_dev.name is passed to the kernel with
write(fd, &uleds_dev, sizeof(uleds_dev)), so a >= LED_MAX_NAME_SIZE
byte argv[1] leaves name without a terminator and the write sends
trailing bytes that are not part of a valid device name.

Force NUL-termination after the strncpy.

Fixes: fa7f32422ea1 ("tools/leds: Add uledmon program for monitoring userspace LEDs")
Signed-off-by: Gu Bin <gubin@cmss.chinamobile.com>
---
 tools/leds/uledmon.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/leds/uledmon.c b/tools/leds/uledmon.c
index c15a39c1f271..6df35b817233 100644
--- a/tools/leds/uledmon.c
+++ b/tools/leds/uledmon.c
@@ -32,6 +32,7 @@ int main(int argc, char const *argv[])
 	}
 
 	strncpy(uleds_dev.name, argv[1], LED_MAX_NAME_SIZE);
+	uleds_dev.name[LED_MAX_NAME_SIZE - 1] = '\0';
 	uleds_dev.max_brightness = 100;
 
 	fd = open("/dev/uleds", O_RDWR);
-- 
2.55.0.windows.3

Re: [PATCH] tools/leds/uledmon: force NUL-termination of device name

[sashiko-bot]

> strncpy() does not NUL-terminate when the source is at least as long
> as the bound. uleds_dev.name is passed to the kernel with
> write(fd, &uleds_dev, sizeof(uleds_dev)), so a >= LED_MAX_NAME_SIZE
> byte argv[1] leaves name without a terminator and the write sends
> trailing bytes that are not part of a valid device name.
> 
> Force NUL-termination after the strncpy.
> 
> Fixes: fa7f32422ea1 ("tools/leds: Add uledmon program for monitoring userspace LEDs")
> Signed-off-by: Gu Bin <gubin@cmss.chinamobile.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261106100614.2312-1-gubin@cmss.chinamobile.com?part=1

Re: [PATCH] tools/leds/uledmon: force NUL-termination of device name

[David Lechner]

On 11/6/26 4:06 AM, Gu Bin wrote:
> strncpy() does not NUL-terminate when the source is at least as long
> as the bound. uleds_dev.name is passed to the kernel with
> write(fd, &uleds_dev, sizeof(uleds_dev)), so a >= LED_MAX_NAME_SIZE
> byte argv[1] leaves name without a terminator and the write sends
> trailing bytes that are not part of a valid device name.
> 
> Force NUL-termination after the strncpy.
> 
> Fixes: fa7f32422ea1 ("tools/leds: Add uledmon program for monitoring userspace LEDs")
> Signed-off-by: Gu Bin <gubin@cmss.chinamobile.com>
> ---
>  tools/leds/uledmon.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/tools/leds/uledmon.c b/tools/leds/uledmon.c
> index c15a39c1f271..6df35b817233 100644
> --- a/tools/leds/uledmon.c
> +++ b/tools/leds/uledmon.c
> @@ -32,6 +32,7 @@ int main(int argc, char const *argv[])
>  	}
>  
>  	strncpy(uleds_dev.name, argv[1], LED_MAX_NAME_SIZE);
> +	uleds_dev.name[LED_MAX_NAME_SIZE - 1] = '\0';
>  	uleds_dev.max_brightness = 100;
>  
>  	fd = open("/dev/uleds", O_RDWR);

This works, but maybe better to just print an error and exit with error code
rather than silently truncating the name?

[PATCH v2] tools/leds/uledmon: reject device names that are too long

[Gu Bin]

strncpy() does not NUL-terminate when the source is at least as long
as the bound. uleds_dev.name is passed to the kernel with
write(fd, &uleds_dev, sizeof(uleds_dev)), so a >= LED_MAX_NAME_SIZE
byte argv[1] leaves name without a terminator and the write sends
trailing bytes that are not part of a valid device name.

Silently truncating such a name would create a device the user did
not ask for, so error out instead, like the other argument checks
already do.

Fixes: fa7f32422ea1 ("tools/leds: Add uledmon program for monitoring userspace LEDs")
Suggested-by: David Lechner <david@lechnology.com>
Signed-off-by: Gu Bin <gubin@cmss.chinamobile.com>
---

Changes in v2: reject over-long names with an error instead of
silently truncating them, as suggested by David Lechner.

 tools/leds/uledmon.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/tools/leds/uledmon.c b/tools/leds/uledmon.c
index c15a39c1f271..d38f5716c7f7 100644
--- a/tools/leds/uledmon.c
+++ b/tools/leds/uledmon.c
@@ -31,7 +31,13 @@ int main(int argc, char const *argv[])
 		return 1;
 	}
 
-	strncpy(uleds_dev.name, argv[1], LED_MAX_NAME_SIZE);
+	if (strlen(argv[1]) >= LED_MAX_NAME_SIZE) {
+		fprintf(stderr, "Device name too long (max %d bytes)\n",
+			LED_MAX_NAME_SIZE - 1);
+		return 1;
+	}
+
+	strcpy(uleds_dev.name, argv[1]);
 	uleds_dev.max_brightness = 100;
 
 	fd = open("/dev/uleds", O_RDWR);
-- 
2.55.0.windows.3

Re: [PATCH v2] tools/leds/uledmon: reject device names that are too long

[sashiko-bot]

> strncpy() does not NUL-terminate when the source is at least as long
> as the bound. uleds_dev.name is passed to the kernel with
> write(fd, &uleds_dev, sizeof(uleds_dev)), so a >= LED_MAX_NAME_SIZE
> byte argv[1] leaves name without a terminator and the write sends
> trailing bytes that are not part of a valid device name.
> 
> Silently truncating such a name would create a device the user did
> not ask for, so error out instead, like the other argument checks
> already do.
> 
> Fixes: fa7f32422ea1 ("tools/leds: Add uledmon program for monitoring userspace LEDs")
> Suggested-by: David Lechner <david@lechnology.com>
> Signed-off-by: Gu Bin <gubin@cmss.chinamobile.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261109070539.1922-1-gubin@cmss.chinamobile.com?part=1