[PATCH v2 1/2] leds: flash: Prevent using uninitialized variable in flash_fault_show()

[PATCH v2 1/2] leds: flash: Prevent using uninitialized variable in flash_fault_show()

[Dmitry Antipov]

In flash_fault_show(), address of uninitialized 'fault' variable is
passed to device-specific .fault_get callbacks. This is not a problem
if such a callback directly assigns the value like rt4505_fault_get(),
but results in an undefined behavior if callback just performs bitwise
ORs like as3645a_get_fault(). So just initialize 'fault' with zero.

Reported-by: sashiko-bot@kernel.org
Closes: https://sashiko.dev/#/patchset/20260904054437.461706-1-dmantipov@yandex.ru?part=1
Fixes: 7aea8389a77ab ("leds: Add LED Flash class extension to the LED subsystem")
Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
---
v2: initial version to join the series
---
 drivers/leds/led-class-flash.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/leds/led-class-flash.c b/drivers/leds/led-class-flash.c
index 165035a8826c..3bb90823de1a 100644
--- a/drivers/leds/led-class-flash.c
+++ b/drivers/leds/led-class-flash.c
@@ -187,7 +187,7 @@ static ssize_t flash_fault_show(struct device *dev,
 {
 	struct led_classdev *led_cdev = dev_get_drvdata(dev);
 	struct led_classdev_flash *fled_cdev = lcdev_to_flcdev(led_cdev);
-	u32 fault, mask = 0x1;
+	u32 fault = 0, mask = 0x1;
 	char *pbuf = buf;
 	int i, ret, buf_len;
 
-- 
2.55.0

[PATCH v2 2/2] 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>
---
v2: capitalize commit title
---
 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 3bb90823de1a..d09c71fcd2b3 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 = 0, 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 v2 2/2] leds: flash: Simplify flash_fault_show()

[sashiko-bot]

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

Sashiko has reviewed this patch and found no issues. It looks great!

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

Re: [PATCH v2 1/2] leds: flash: Prevent using uninitialized variable in flash_fault_show()

[sashiko-bot]

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

Pre-existing issues:
- [High] Use-after-free due to missing indicator LED unregistration on probe error path.
- [Medium] Accumulating fault state in V4L2 flash controls due to uncleared variable.
--

commit 9123f63288afc4cb1ae08569252a324f5721a8b4
Author: Dmitry Antipov <dmantipov@yandex.ru>

leds: flash: Prevent using uninitialized variable in flash_fault_show()

This commit initializes the fault variable in flash_fault_show() to prevent
undefined behavior. This occurs when driver callbacks only perform bitwise
ORs without clearing the pointer first.

Closes: https://sashiko.dev/#/patchset/20260904054437.461706-1-dmantipov@yandex.ru?part=1

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

[Severity: Medium]
This is a pre-existing issue, but while this commit fixes the
uninitialized variable in flash_fault_show(), does the exact same issue
exist in the V4L2 flash controls?

In drivers/media/v4l2-core/v4l2-flash-led-class.c:v4l2_flash_g_volatile_ctrl():

    case V4L2_CID_FLASH_FAULT:
        /* LED faults map directly to V4L2 flash faults */
        return led_get_flash_fault(fled_cdev, &c->val);

If a user-space process issues a volatile read for V4L2_CID_FLASH_FAULT,
c->val retains its previously cached value. Because underlying callbacks
like as3645a_get_fault() in drivers/leds/flash/leds-as3645a.c only
perform bitwise OR operations without clearing the pointer:

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

cleared hardware faults will permanently remain stuck in the reported
V4L2 state until the module is reloaded. Could c->val be cleared before
calling led_get_flash_fault() here as well?

[Severity: High]
This is a pre-existing issue, but looking at the as3645a driver
mentioned above, is there a memory corruption concern on its probe error
path due to missing unregistration of the indicator LED?

In drivers/leds/flash/leds-as3645a.c:as3645a_probe():

    out_led_classdev_flash_unregister:
        led_classdev_flash_unregister(&flash->fled);

    out_mutex_destroy:

If as3645a_v4l2_setup() fails during device probe, the error path
unregisters the flash LED but completely misses unregistering the
indicator LED, iled_cdev, which was manually registered in
as3645a_led_class_setup().

The probe function returns an error, causing the devres framework to
automatically free the devm-allocated flash driver data structure. The
indicator LED remains fully registered and exposed to user-space sysfs.
Accessing its sysfs attributes will dereference the freed flash struct,
leading to a use-after-free. Would it make sense to call
led_classdev_unregister() for the indicator LED in this error path?

>  	char *pbuf = buf;
>  	int i, ret, buf_len;

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