[PATCH v2] leds: flash: leds-qcom-flash: Release all V4L2 flash devices

[PATCH v2] leds: flash: leds-qcom-flash: Release all V4L2 flash devices

[George Neacsu via B4 Relay]

From: George Neacsu <geocausa@gmail.com>

The probe error path and qcom_flash_led_remove() use leds_count as an
array index before decrementing it. Since leds_count is the number of
registered LEDs, that first access is one entry past the last registered
V4L2 flash device. The loops also stop before index 0, so with a single
LED nothing is released at all.

The V4L2 flash sub-device then stays registered after the driver is
unbound, still pointing at the driver's freed devm memory. On a Surface
Pro 11 (X1E80100, PM8550 flash, IR LED linked to a sensor through
flash-leds), unloading the module and then setting a flash control on
the sub-device triggered UBSAN shift-out-of-bounds reports and a NULL
pointer dereference in regmap_field_update_bits_base(), called from
update_allowed_flash_current().

Release the registered entries from leds_count - 1 down to 0. Initialize
the mutex only after the probe checks that can return directly, destroy
it on the shared failure path, and route a final torch-clamp programming
failure through that cleanup as well.

Fixes: 96a2e242a5dc ("leds: flash: Add driver to support flash LED module in QCOM PMICs")
Cc: stable@vger.kernel.org
Reported-by: Sashiko AI review <sashiko-bot@kernel.org>
Link: https://sashiko.dev/#/patchset/20261016-qcom-flash-v4l2-release-v1-1-0e92aa7c9327@gmail.com?part=1
Assisted-by: Claude:claude-opus-5-5
Assisted-by: ChatGPT:gpt-5.6-sol
Signed-off-by: George Neacsu <geocausa@gmail.com>
---
The V4L2 flash cleanup currently indexes one element past the registered
array and fails to release index 0. Fix the cleanup for both probe failure
and driver removal.

v2 also makes the probe error handling symmetric around the mutex and
routes a late torch-clamp programming failure through the V4L2 cleanup.
---
Changes in v2:
- initialize the mutex after direct-return probe checks and destroy it on
  the shared failure path, addressing the Sashiko review;
- route the final torch-clamp register-write failure through V4L2 cleanup;
- retain the v1 tested cleanup of every registered V4L2 flash device.
- Link to v1: https://lore.kernel.org/r/20261016-qcom-flash-v4l2-release-v1-1-0e92aa7c9327@gmail.com
---
 drivers/leds/flash/leds-qcom-flash.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/drivers/leds/flash/leds-qcom-flash.c b/drivers/leds/flash/leds-qcom-flash.c
index b03a683..35646c4 100644
--- a/drivers/leds/flash/leds-qcom-flash.c
+++ b/drivers/leds/flash/leds-qcom-flash.c
@@ -923,7 +923,6 @@ static int qcom_flash_led_probe(struct platform_device *pdev)
 	devm_kfree(dev, regs); /* devm_regmap_field_bulk_alloc() makes copies */
 
 	platform_set_drvdata(pdev, flash_data);
-	mutex_init(&flash_data->lock);
 
 	count = device_get_child_node_count(dev);
 	if (count == 0 || count > flash_data->max_channels) {
@@ -936,6 +935,8 @@ static int qcom_flash_led_probe(struct platform_device *pdev)
 	if (!flash_data->v4l2_flash)
 		return -ENOMEM;
 
+	mutex_init(&flash_data->lock);
+
 	device_for_each_child_node_scoped(dev, child) {
 		led = devm_kzalloc(dev, sizeof(*led), GFP_KERNEL);
 		if (!led) {
@@ -951,10 +952,16 @@ static int qcom_flash_led_probe(struct platform_device *pdev)
 		flash_data->leds_count++;
 	}
 
-	return regmap_field_write(flash_data->r_fields[REG_TORCH_CLAMP], flash_data->torch_clamp);
+	rc = regmap_field_write(flash_data->r_fields[REG_TORCH_CLAMP],
+				flash_data->torch_clamp);
+	if (rc)
+		goto release;
+
+	return 0;
 release:
-	while (flash_data->v4l2_flash[flash_data->leds_count] && flash_data->leds_count)
-		v4l2_flash_release(flash_data->v4l2_flash[flash_data->leds_count--]);
+	while (flash_data->leds_count)
+		v4l2_flash_release(flash_data->v4l2_flash[--flash_data->leds_count]);
+	mutex_destroy(&flash_data->lock);
 	return rc;
 }
 
@@ -962,8 +969,8 @@ static void qcom_flash_led_remove(struct platform_device *pdev)
 {
 	struct qcom_flash_data *flash_data = platform_get_drvdata(pdev);
 
-	while (flash_data->v4l2_flash[flash_data->leds_count] && flash_data->leds_count)
-		v4l2_flash_release(flash_data->v4l2_flash[flash_data->leds_count--]);
+	while (flash_data->leds_count)
+		v4l2_flash_release(flash_data->v4l2_flash[--flash_data->leds_count]);
 
 	mutex_destroy(&flash_data->lock);
 }

---
base-commit: 62f482ff73bf3ae88b0c899115527752cf9cbf13
change-id: 20261016-qcom-flash-v4l2-release-9d05c97d1c47

Best regards,
-- 
George Neacsu <geocausa@gmail.com>

[PATCH v2] leds: flash: leds-qcom-flash: Release all V4L2 flash devices

[George Neacsu]

The probe error path and qcom_flash_led_remove() use leds_count as an
array index before decrementing it. Since leds_count is the number of
registered LEDs, that first access is one entry past the last registered
V4L2 flash device. The loops also stop before index 0, so with a single
LED nothing is released at all.

The V4L2 flash sub-device then stays registered after the driver is
unbound, still pointing at the driver's freed devm memory. On a Surface
Pro 11 (X1E80100, PM8550 flash, IR LED linked to a sensor through
flash-leds), unloading the module and then setting a flash control on
the sub-device triggered UBSAN shift-out-of-bounds reports and a NULL
pointer dereference in regmap_field_update_bits_base(), called from
update_allowed_flash_current().

Release the registered entries from leds_count - 1 down to 0. Initialize
the mutex only after the probe checks that can return directly, destroy
it on the shared failure path, and route a final torch-clamp programming
failure through that cleanup as well.

Fixes: 96a2e242a5dc ("leds: flash: Add driver to support flash LED module in QCOM PMICs")
Cc: stable@vger.kernel.org
Reported-by: Sashiko AI review <sashiko-bot@kernel.org>
Link: https://sashiko.dev/#/patchset/20261016-qcom-flash-v4l2-release-v1-1-0e92aa7c9327@gmail.com?part=1
Assisted-by: Claude:claude-opus-5-5
Assisted-by: ChatGPT:gpt-5.6-sol
Signed-off-by: George Neacsu <geocausa@gmail.com>
---
The V4L2 flash cleanup currently indexes one element past the registered
array and fails to release index 0. Fix the cleanup for both probe failure
and driver removal.

v2 also makes the probe error handling symmetric around the mutex and
routes a late torch-clamp programming failure through the V4L2 cleanup.
---
Changes in v2:
- initialize the mutex after direct-return probe checks and destroy it on
  the shared failure path, addressing the Sashiko review;
- route the final torch-clamp register-write failure through V4L2 cleanup;
- retain the v1 tested cleanup of every registered V4L2 flash device.
- Link to v1: https://lore.kernel.org/r/20261016-qcom-flash-v4l2-release-v1-1-0e92aa7c9327@gmail.com
---
 drivers/leds/flash/leds-qcom-flash.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/drivers/leds/flash/leds-qcom-flash.c b/drivers/leds/flash/leds-qcom-flash.c
index b03a683..35646c4 100644
--- a/drivers/leds/flash/leds-qcom-flash.c
+++ b/drivers/leds/flash/leds-qcom-flash.c
@@ -923,7 +923,6 @@ static int qcom_flash_led_probe(struct platform_device *pdev)
 	devm_kfree(dev, regs); /* devm_regmap_field_bulk_alloc() makes copies */
 
 	platform_set_drvdata(pdev, flash_data);
-	mutex_init(&flash_data->lock);
 
 	count = device_get_child_node_count(dev);
 	if (count == 0 || count > flash_data->max_channels) {
@@ -936,6 +935,8 @@ static int qcom_flash_led_probe(struct platform_device *pdev)
 	if (!flash_data->v4l2_flash)
 		return -ENOMEM;
 
+	mutex_init(&flash_data->lock);
+
 	device_for_each_child_node_scoped(dev, child) {
 		led = devm_kzalloc(dev, sizeof(*led), GFP_KERNEL);
 		if (!led) {
@@ -951,10 +952,16 @@ static int qcom_flash_led_probe(struct platform_device *pdev)
 		flash_data->leds_count++;
 	}
 
-	return regmap_field_write(flash_data->r_fields[REG_TORCH_CLAMP], flash_data->torch_clamp);
+	rc = regmap_field_write(flash_data->r_fields[REG_TORCH_CLAMP],
+				flash_data->torch_clamp);
+	if (rc)
+		goto release;
+
+	return 0;
 release:
-	while (flash_data->v4l2_flash[flash_data->leds_count] && flash_data->leds_count)
-		v4l2_flash_release(flash_data->v4l2_flash[flash_data->leds_count--]);
+	while (flash_data->leds_count)
+		v4l2_flash_release(flash_data->v4l2_flash[--flash_data->leds_count]);
+	mutex_destroy(&flash_data->lock);
 	return rc;
 }
 
@@ -962,8 +969,8 @@ static void qcom_flash_led_remove(struct platform_device *pdev)
 {
 	struct qcom_flash_data *flash_data = platform_get_drvdata(pdev);
 
-	while (flash_data->v4l2_flash[flash_data->leds_count] && flash_data->leds_count)
-		v4l2_flash_release(flash_data->v4l2_flash[flash_data->leds_count--]);
+	while (flash_data->leds_count)
+		v4l2_flash_release(flash_data->v4l2_flash[--flash_data->leds_count]);
 
 	mutex_destroy(&flash_data->lock);
 }

---
base-commit: 62f482ff73bf3ae88b0c899115527752cf9cbf13
change-id: 20261016-qcom-flash-v4l2-release-9d05c97d1c47

Best regards,
-- 
George Neacsu <geocausa@gmail.com>

Re: [PATCH v2] leds: flash: leds-qcom-flash: Release all V4L2 flash devices

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] leds: qcom-flash: mutex_destroy() called before devm LED unregister
--

commit a8ffc86d81428f44e3211062ba0e1f32e17c9078
Author: George Neacsu <geocausa@gmail.com>

leds: flash: leds-qcom-flash: Release all V4L2 flash devices

This commit fixes the V4L2 flash release loop in both the probe error path
and qcom_flash_led_remove() so that all registered entries from
leds_count - 1 down to 0 are properly released. It also moves mutex_init()
after the early probe checks, destroys the mutex on the failure path, and
routes a torch-clamp register write failure through the release label.

Link: https://sashiko.dev/#/patchset/20261016-qcom-flash-v4l2-release-v1-1-0e92aa7c9327@gmail.com?part=1

> diff --git a/drivers/leds/flash/leds-qcom-flash.c b/drivers/leds/flash/leds-qcom-flash.c
> index b03a6833e3e3a..35646c4b839dd 100644
> --- a/drivers/leds/flash/leds-qcom-flash.c
> +++ b/drivers/leds/flash/leds-qcom-flash.c
> @@ -923,7 +923,6 @@ static int qcom_flash_led_probe(struct platform_device *pdev)
>  	devm_kfree(dev, regs); /* devm_regmap_field_bulk_alloc() makes copies */
>  
>  	platform_set_drvdata(pdev, flash_data);
> -	mutex_init(&flash_data->lock);
>  
>  	count = device_get_child_node_count(dev);
>  	if (count == 0 || count > flash_data->max_channels) {
> @@ -936,6 +935,8 @@ static int qcom_flash_led_probe(struct platform_device *pdev)
>  	if (!flash_data->v4l2_flash)
>  		return -ENOMEM;
>  
> +	mutex_init(&flash_data->lock);
> +
>  	device_for_each_child_node_scoped(dev, child) {
>  		led = devm_kzalloc(dev, sizeof(*led), GFP_KERNEL);
>  		if (!led) {
> @@ -951,10 +952,16 @@ static int qcom_flash_led_probe(struct platform_device *pdev)
>  		flash_data->leds_count++;
>  	}
>  
> -	return regmap_field_write(flash_data->r_fields[REG_TORCH_CLAMP], flash_data->torch_clamp);
> +	rc = regmap_field_write(flash_data->r_fields[REG_TORCH_CLAMP],
> +				flash_data->torch_clamp);
> +	if (rc)
> +		goto release;
> +
> +	return 0;
>  release:
> -	while (flash_data->v4l2_flash[flash_data->leds_count] && flash_data->leds_count)
> -		v4l2_flash_release(flash_data->v4l2_flash[flash_data->leds_count--]);
> +	while (flash_data->leds_count)
> +		v4l2_flash_release(flash_data->v4l2_flash[--flash_data->leds_count]);
> +	mutex_destroy(&flash_data->lock);
>  	return rc;
>  }
>  
> @@ -962,8 +969,8 @@ static void qcom_flash_led_remove(struct platform_device *pdev)
>  {
>  	struct qcom_flash_data *flash_data = platform_get_drvdata(pdev);
>  
> -	while (flash_data->v4l2_flash[flash_data->leds_count] && flash_data->leds_count)
> -		v4l2_flash_release(flash_data->v4l2_flash[flash_data->leds_count--]);
> +	while (flash_data->leds_count)
> +		v4l2_flash_release(flash_data->v4l2_flash[--flash_data->leds_count]);
>  
>  	mutex_destroy(&flash_data->lock);
>  }

[Severity: High]
Does calling mutex_destroy(&flash_data->lock) in the qcom_flash_led_probe()
error path and in qcom_flash_led_remove() destroy the mutex while managed
LED class devices can still acquire it?

In qcom_flash_register_led_device(), each LED registers a managed flash
classdev before qcom_flash_v4l2_init() is called:

    flash->led_cdev.brightness_set_blocking = qcom_flash_led_brightness_set;
    ...
    rc = devm_led_classdev_flash_register_ext(dev, flash, &init_data);

Because devm_led_classdev_flash_register_ext() is devres-managed, its
cleanup runs during devres_release_all(), which happens after
qcom_flash_led_probe() returns an error or after qcom_flash_led_remove()
completes.

When devres_release_all() unregisters the LED via
devm_led_classdev_flash_release() -> led_classdev_flash_unregister() ->
led_classdev_unregister(), it turns off the LED and flushes
set_brightness_work:

    if (!(led_cdev->flags & LED_RETAIN_AT_SHUTDOWN))
        led_set_brightness(led_cdev, LED_OFF);

    flush_work(&led_cdev->set_brightness_work);

Flushing set_brightness_work invokes qcom_flash_led_brightness_set() ->
set_flash_module_en() and update_allowed_flash_current(), both of which
call mutex_lock(&flash_data->lock) and mutex_unlock(&flash_data->lock) on
the mutex that was already destroyed.

Would using devm_mutex_init(dev, &flash_data->lock) before the child node
registration loop (and dropping the manual mutex_destroy() calls) ensure
the mutex is destroyed by devres only after all managed LED devices have
been unregistered?

титься

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261018-qcom-flash-v4l2-release-v2-1-8e2297844d9f@gmail.com?part=1