[PATCH] leds: flash: simplify flash_fault_show()

[PATCH] leds: flash: simplify flash_fault_show()

[Dmitry Antipov]

Do not emit extra trailing space and prefer direct pointer
ops over strcat() and strlen() calls in flash_fault_show().

Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
---
 drivers/leds/led-class-flash.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/drivers/leds/led-class-flash.c b/drivers/leds/led-class-flash.c
index 165035a8826c..1e878baff041 100644
--- a/drivers/leds/led-class-flash.c
+++ b/drivers/leds/led-class-flash.c
@@ -189,24 +189,22 @@ static ssize_t flash_fault_show(struct device *dev,
 	struct led_classdev_flash *fled_cdev = lcdev_to_flcdev(led_cdev);
 	u32 fault, mask = 0x1;
 	char *pbuf = buf;
-	int i, ret, buf_len;
+	int i, ret;
 
 	ret = led_get_flash_fault(fled_cdev, &fault);
 	if (ret < 0)
 		return -EINVAL;
 
-	*buf = '\0';
-
 	for (i = 0; i < LED_NUM_FLASH_FAULTS; ++i) {
-		if (fault & mask) {
-			buf_len = sprintf(pbuf, "%s ",
-					  led_flash_fault_names[i]);
-			pbuf += buf_len;
-		}
+		if (fault & mask)
+			pbuf += sprintf(pbuf, "%s%s",
+					(pbuf == buf ? "" : " "),
+					led_flash_fault_names[i]);
 		mask <<= 1;
 	}
-
-	return strlen(strcat(buf, "\n"));
+	*pbuf++ = '\n';
+	*pbuf = '\0';
+	return pbuf - buf;
 }
 static DEVICE_ATTR_RO(flash_fault);
 
-- 
2.55.0

Re: [PATCH] leds: flash: simplify flash_fault_show()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Low] The commit message description is not capitalized, violating the LED subsystem guidelines.

Pre-existing issues:
- [High] Uninitialized stack variable `fault` leaks kernel memory state to userspace.
--

commit 57bfe07a03cad5bedb847332afd85707164afd8d
Author: Dmitry Antipov <dmantipov@yandex.ru>

leds: flash: simplify flash_fault_show()

Do not emit extra trailing space and prefer direct pointer
ops over strcat() and strlen() calls in flash_fault_show().

> leds: flash: simplify flash_fault_show()

[Severity: Low]
Does this commit subject violate the LED subsystem naming convention?

The guidelines state that the description after the subsystem prefix should
always be capitalized. Could this be changed to something like
"leds: flash: Simplify flash_fault_show()"?

> diff --git a/drivers/leds/led-class-flash.c b/drivers/leds/led-class-flash.c
> index 6fe9d700dfef6..cc9600a311296 100644
> --- a/drivers/leds/led-class-flash.c
> +++ b/drivers/leds/led-class-flash.c
> @@ -190,24 +190,22 @@ static ssize_t flash_fault_show(struct device *dev,
>  	struct led_classdev_flash *fled_cdev = lcdev_to_flcdev(led_cdev);
>  	u32 fault, mask = 0x1;

[Severity: High]
This isn't a bug introduced by this patch, but does this leak uninitialized
kernel stack memory to userspace?

When reading the flash_fault sysfs attribute, flash_fault_show() allocates
the fault variable on the stack without initializing it, and then passes it
by reference:

	ret = led_get_flash_fault(fled_cdev, &fault);

If the driver's fault_get callback (such as as3645a_get_fault() in
leds-as3645a.c) uses bitwise OR without first initializing the variable, it
preserves the stack garbage:

	if (rval & AS_FAULT_INFO_INDUCTOR_PEAK_LIMIT)
		*fault |= LED_FAULT_OVER_CURRENT;

The loop in flash_fault_show() then iterates over the 9 lowest bits of this
value and prints their corresponding fault names, which could disclose
kernel memory to unprivileged userspace processes.

Could the fault variable be explicitly initialized to 0 during declaration?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260904054437.461706-1-dmantipov@yandex.ru?part=1