[PATCH v2] leds: lp5860: fix init error-path locking

[PATCH v2] leds: lp5860: fix init error-path locking

[kr494167]

From: surendra <kr494167@gmail.com>

lp5860_device_init() unlocks lp->lock after updating the device mode.
An lp5860_init_dt() failure then jumps to err_disable, which attempts to
unlock the mutex a second time before disabling the chip.

Unlock immediately after the register update and keep the common error
handler lockless.

Fixes: f0a66563aa2d ("leds: Add support for TI LP5860 LED driver chip")

Signed-off-by: surendra <kr494167@gmail.com>
---
 drivers/leds/rgb/leds-lp5860-core.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/leds/rgb/leds-lp5860-core.c b/drivers/leds/rgb/leds-lp5860-core.c
index fd0e2f6e6e0f..e21d5f2302be 100644
--- a/drivers/leds/rgb/leds-lp5860-core.c
+++ b/drivers/leds/rgb/leds-lp5860-core.c
@@ -204,9 +204,9 @@ int lp5860_device_init(struct device *dev)
 	mutex_lock(&lp->lock);
 	ret = regmap_update_bits(lp->regmap, LP5860_REG_DEV_INITIAL, LP5860_MODE_MASK,
 				 LP5860_MODE_1 << LP5860_MODE_SHIFT);
+	mutex_unlock(&lp->lock);
 	if (ret)
 		goto err_disable;
-	mutex_unlock(&lp->lock);
 
 	ret = lp5860_init_dt(lp);
 	if (ret)
@@ -215,7 +215,6 @@ int lp5860_device_init(struct device *dev)
 	return 0;
 
 err_disable:
-	mutex_unlock(&lp->lock);
 	lp5860_chip_enable(lp, LP5860_CHIP_DISABLE);
 	return ret;
 }
-- 
2.55.0

Re: [PATCH v2] leds: lp5860: fix init error-path locking

[sashiko-bot]

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

New issues:
- [Low] The commit message violates the LED subsystem capitalization guidelines.

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but the driver explicitly destroys its mutex in `.remove()` before hardware teardown completes and before managed LED resources are unregistered, leading to a deterministic use-after-destroy vulnerability.
--

--- Patch [1]: [PATCH v2] leds: lp5860: fix init error-path locking ---
commit 685affba31d70c4dc3416357a81ff413cf7d2165
Author: surendra <kr494167@gmail.com>

leds: lp5860: fix init error-path locking

This commit fixes a double-unlock issue in the lp5860 initialization
error path. By unlocking immediately after the register update, the
common error handler remains lockless.

> leds: lp5860: fix init error-path locking

[Severity: Low]
This isn't a bug, but the commit message violates the LED subsystem 
capitalization guidelines which mandate capitalizing the description after 
the subsystem prefix. Should this be "leds: lp5860: Fix init error-path 
locking"?

> diff --git a/drivers/leds/rgb/leds-lp5860-core.c b/drivers/leds/rgb/leds-lp5860-core.c
> index fd0e2f6e6e0f4..e21d5f2302be0 100644
> --- a/drivers/leds/rgb/leds-lp5860-core.c
> +++ b/drivers/leds/rgb/leds-lp5860-core.c

[ ... ]

> @@ -215,7 +215,6 @@ int lp5860_device_init(struct device *dev)
>  	return 0;
>  
>  err_disable:
> -	mutex_unlock(&lp->lock);
>  	lp5860_chip_enable(lp, LP5860_CHIP_DISABLE);

[Severity: High]
This isn't a bug introduced by this patch, but looking at the teardown
path, is there a use-after-destroy vulnerability with this mutex?

When the device is removed, lp5860_remove() in 
drivers/leds/rgb/leds-lp5860-spi.c explicitly calls:

mutex_destroy(&lp5860->lock);

It then calls lp5860_device_remove() which disables the chip by calling 
lp5860_chip_enable(). That function attempts to lock the destroyed mutex:

static int lp5860_chip_enable(struct lp5860 *lp, bool enable)
{
	guard(mutex)(&lp->lock);
	return regmap_write(lp->regmap, LP5860_REG_CHIP_EN, enable);
}

Additionally, the managed LED class devices are unregistered later by devres,
which invokes lp5860_set_mc_brightness(). This also attempts to lock the
already destroyed mutex via guard(mutex)(&led->chip->lock). 

Can this sequence cause a lockdep splat or kernel panic during unbind?

>  	return ret;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720104838.98101-1-kr494167@gmail.com?part=1