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

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 <[email protected]>

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/[email protected]?part=1