[PATCH 1/2] leds: lm3530: use brightness_set_blocking for sleepable callback

[PATCH 1/2] leds: lm3530: use brightness_set_blocking for sleepable callback

[Runyu Xiao]

lm3530_brightness_set() talks to the device over I2C and can sleep, but
the driver registers it as brightness_set.  That leaves the LED core
free to invoke the callback from atomic contexts.

This issue was found by our static analysis tool and then manually
reviewed against the current tree.

A minimal Lockdep reproducer that keeps the original registration and
call chain is enough to trigger the warning: lm3530_probe() still
publishes lm3530_brightness_set() as brightness_set,
led_trigger_event_atomic() invokes it under spin_lock_irqsave(), and
the callback reaches i2c_smbus_write_byte_data() as its first
sleepable edge.

Lockdep reports:

  BUG: sleeping function called from invalid context
  i2c_smbus_write_byte_data.constprop.0+0x14/0x30 [vuln_msv]
  lm3530_brightness_set+0x4e/0x66 [vuln_msv]
  led_trigger_event_atomic.constprop.0+0x2b/0x40 [vuln_msv]

Convert the callback to brightness_set_blocking so the LED core only
invokes it from a sleepable context.

Fixes: b1e6b7068f02 ("leds: add driver for LM3530 ALS")
Cc: stable@vger.kernel.org
Signed-off-by: Runyu Xiao <runyu.xiao@seu.edu.cn>
---
Notes:
  - Validated with a grounded Lockdep PoC that preserves the
    lm3530_probe() brightness_set registration and the
    led_trigger_event_atomic() -> lm3530_brightness_set() ->
    i2c_smbus_write_byte_data() path.
  - checkpatch.pl --strict: clean.
  - Not tested on LM3530 hardware.

 drivers/leds/leds-lm3530.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/leds/leds-lm3530.c b/drivers/leds/leds-lm3530.c
index e44a3db106c3..ba744961ebcd 100644
--- a/drivers/leds/leds-lm3530.c
+++ b/drivers/leds/leds-lm3530.c
@@ -301,10 +301,11 @@ static int lm3530_init_registers(struct lm3530_data *drvdata)
 	return ret;
 }
 
-static void lm3530_brightness_set(struct led_classdev *led_cdev,
-				     enum led_brightness brt_val)
+static int
+lm3530_brightness_set_blocking(struct led_classdev *led_cdev,
+			       enum led_brightness brt_val)
 {
-	int err;
+	int err = 0;
 	struct lm3530_data *drvdata =
 	    container_of(led_cdev, struct lm3530_data, led_dev);
 	struct lm3530_platform_data *pdata = drvdata->pdata;
@@ -344,6 +345,8 @@ static void lm3530_brightness_set(struct led_classdev *led_cdev,
 	default:
 		break;
 	}
+
+	return err;
 }
 
 static ssize_t mode_show(struct device *dev,
@@ -438,7 +441,8 @@ static int lm3530_probe(struct i2c_client *client)
 	drvdata->brightness = LED_OFF;
 	drvdata->enable = false;
 	drvdata->led_dev.name = LM3530_LED_DEV;
-	drvdata->led_dev.brightness_set = lm3530_brightness_set;
+	drvdata->led_dev.brightness_set_blocking =
+		lm3530_brightness_set_blocking;
 	drvdata->led_dev.max_brightness = MAX_BRIGHTNESS;
 	drvdata->led_dev.groups = lm3530_groups;
 
-- 
2.34.1

Re: [PATCH 1/2] leds: lm3530: use brightness_set_blocking for sleepable callback

[Lee Jones]

On Mon, 15 Jun 2026, Runyu Xiao wrote:

> lm3530_brightness_set() talks to the device over I2C and can sleep, but
> the driver registers it as brightness_set.  That leaves the LED core
> free to invoke the callback from atomic contexts.
> 
> This issue was found by our static analysis tool and then manually
> reviewed against the current tree.
> 
> A minimal Lockdep reproducer that keeps the original registration and
> call chain is enough to trigger the warning: lm3530_probe() still
> publishes lm3530_brightness_set() as brightness_set,
> led_trigger_event_atomic() invokes it under spin_lock_irqsave(), and
> the callback reaches i2c_smbus_write_byte_data() as its first
> sleepable edge.
> 
> Lockdep reports:
> 
>   BUG: sleeping function called from invalid context
>   i2c_smbus_write_byte_data.constprop.0+0x14/0x30 [vuln_msv]
>   lm3530_brightness_set+0x4e/0x66 [vuln_msv]
>   led_trigger_event_atomic.constprop.0+0x2b/0x40 [vuln_msv]
> 
> Convert the callback to brightness_set_blocking so the LED core only
> invokes it from a sleepable context.
> 
> Fixes: b1e6b7068f02 ("leds: add driver for LM3530 ALS")
> Cc: stable@vger.kernel.org
> Signed-off-by: Runyu Xiao <runyu.xiao@seu.edu.cn>
> ---
> Notes:
>   - Validated with a grounded Lockdep PoC that preserves the
>     lm3530_probe() brightness_set registration and the
>     led_trigger_event_atomic() -> lm3530_brightness_set() ->
>     i2c_smbus_write_byte_data() path.
>   - checkpatch.pl --strict: clean.
>   - Not tested on LM3530 hardware.
> 
>  drivers/leds/leds-lm3530.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/leds/leds-lm3530.c b/drivers/leds/leds-lm3530.c
> index e44a3db106c3..ba744961ebcd 100644
> --- a/drivers/leds/leds-lm3530.c
> +++ b/drivers/leds/leds-lm3530.c
> @@ -301,10 +301,11 @@ static int lm3530_init_registers(struct lm3530_data *drvdata)
>  	return ret;
>  }
>  
> -static void lm3530_brightness_set(struct led_classdev *led_cdev,
> -				     enum led_brightness brt_val)
> +static int
> +lm3530_brightness_set_blocking(struct led_classdev *led_cdev,
> +			       enum led_brightness brt_val)
>  {
> -	int err;
> +	int err = 0;
>  	struct lm3530_data *drvdata =
>  	    container_of(led_cdev, struct lm3530_data, led_dev);
>  	struct lm3530_platform_data *pdata = drvdata->pdata;
> @@ -344,6 +345,8 @@ static void lm3530_brightness_set(struct led_classdev *led_cdev,
>  	default:
>  		break;
>  	}
> +
> +	return err;

What makes you think that the original author didn't want these failures
to be non-catastrophic?

>  }
>  
>  static ssize_t mode_show(struct device *dev,
> @@ -438,7 +441,8 @@ static int lm3530_probe(struct i2c_client *client)
>  	drvdata->brightness = LED_OFF;
>  	drvdata->enable = false;
>  	drvdata->led_dev.name = LM3530_LED_DEV;
> -	drvdata->led_dev.brightness_set = lm3530_brightness_set;
> +	drvdata->led_dev.brightness_set_blocking =
> +		lm3530_brightness_set_blocking;

We've had no complaints about this in the 15-years it's been
operational.  I think it's safe to conclude that this isn't causes
anyone issues.  I'd be concerned that these changes, although seemingly
correct on the surface, may cause problems for users, which would be
unacceptable.

>  	drvdata->led_dev.max_brightness = MAX_BRIGHTNESS;
>  	drvdata->led_dev.groups = lm3530_groups;
>  
> -- 
> 2.34.1

-- 
Lee Jones

[PATCH 2/2] leds: menf21bmc: use brightness_set_blocking for sleepable callback

[Runyu Xiao]

menf21bmc_led_set() serializes access with a mutex and performs I2C
transfers, but the driver exposes it through brightness_set.  That
mismatches the LED core callback contract for atomic callers.

This issue was found by our static analysis tool and then manually
reviewed against the current tree.

A minimal Lockdep reproducer that keeps the original registration and
call chain is enough to trigger the warning: menf21bmc_led_probe()
still publishes menf21bmc_led_set() as brightness_set,
led_trigger_event_atomic() invokes it under spin_lock_irqsave(), and
the callback immediately tries mutex_lock(&led_lock) before reaching
its I2C accesses.

Lockdep reports:

  BUG: sleeping function called from invalid context
  __mutex_lock+0x4f/0xd20
  menf21bmc_led_set+0x15/0x49 [vuln_msv]
  [ BUG: Invalid wait context ]
  ... (led_lock#2) ... at: menf21bmc_led_set+0x15/0x49 [vuln_msv]

Convert the driver to brightness_set_blocking and return the transport
status to the LED core.

Fixes: 38433639af91 ("leds: leds-menf21bmc: Introduce MEN 14F021P00 BMC LED driver")
Cc: stable@vger.kernel.org
Signed-off-by: Runyu Xiao <runyu.xiao@seu.edu.cn>
---
Notes:
  - Validated with a grounded Lockdep PoC that preserves the
    menf21bmc_led_probe() brightness_set registration and the
    led_trigger_event_atomic() -> menf21bmc_led_set() ->
    mutex_lock(&led_lock) path.
  - checkpatch.pl --strict: clean.
  - Not tested on MEN 14F021P00 BMC hardware.

 drivers/leds/leds-menf21bmc.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/leds/leds-menf21bmc.c b/drivers/leds/leds-menf21bmc.c
index 6b1b47160602..da476fe6ed2c 100644
--- a/drivers/leds/leds-menf21bmc.c
+++ b/drivers/leds/leds-menf21bmc.c
@@ -49,28 +49,33 @@ static struct menf21bmc_led leds[] = {
 
 static DEFINE_MUTEX(led_lock);
 
-static void
-menf21bmc_led_set(struct led_classdev *led_cdev, enum led_brightness value)
+static int
+menf21bmc_led_set_blocking(struct led_classdev *led_cdev,
+			   enum led_brightness value)
 {
 	int led_val;
+	int ret = 0;
 	struct menf21bmc_led *led = container_of(led_cdev,
 					struct menf21bmc_led, cdev);
 
 	mutex_lock(&led_lock);
 	led_val = i2c_smbus_read_byte_data(led->i2c_client,
 					   BMC_CMD_LED_GET_SET);
-	if (led_val < 0)
+	if (led_val < 0) {
+		ret = led_val;
 		goto err_out;
+	}
 
 	if (value == LED_OFF)
 		led_val &= ~led->led_bit;
 	else
 		led_val |= led->led_bit;
 
-	i2c_smbus_write_byte_data(led->i2c_client,
-				  BMC_CMD_LED_GET_SET, led_val);
+	ret = i2c_smbus_write_byte_data(led->i2c_client,
+					BMC_CMD_LED_GET_SET, led_val);
 err_out:
 	mutex_unlock(&led_lock);
+	return ret;
 }
 
 static int menf21bmc_led_probe(struct platform_device *pdev)
@@ -81,7 +86,8 @@ static int menf21bmc_led_probe(struct platform_device *pdev)
 
 	for (i = 0; i < ARRAY_SIZE(leds); i++) {
 		leds[i].cdev.name = leds[i].name;
-		leds[i].cdev.brightness_set = menf21bmc_led_set;
+		leds[i].cdev.brightness_set_blocking =
+			menf21bmc_led_set_blocking;
 		leds[i].i2c_client = i2c_client;
 		ret = devm_led_classdev_register(&pdev->dev, &leds[i].cdev);
 		if (ret < 0) {
-- 
2.34.1

Re: [PATCH 2/2] leds: menf21bmc: use brightness_set_blocking for sleepable callback

[Lee Jones]

Andreas, any thoughts?

On Mon, 15 Jun 2026, Runyu Xiao wrote:

> menf21bmc_led_set() serializes access with a mutex and performs I2C
> transfers, but the driver exposes it through brightness_set.  That
> mismatches the LED core callback contract for atomic callers.
> 
> This issue was found by our static analysis tool and then manually
> reviewed against the current tree.
> 
> A minimal Lockdep reproducer that keeps the original registration and
> call chain is enough to trigger the warning: menf21bmc_led_probe()
> still publishes menf21bmc_led_set() as brightness_set,
> led_trigger_event_atomic() invokes it under spin_lock_irqsave(), and
> the callback immediately tries mutex_lock(&led_lock) before reaching
> its I2C accesses.
> 
> Lockdep reports:
> 
>   BUG: sleeping function called from invalid context
>   __mutex_lock+0x4f/0xd20
>   menf21bmc_led_set+0x15/0x49 [vuln_msv]
>   [ BUG: Invalid wait context ]
>   ... (led_lock#2) ... at: menf21bmc_led_set+0x15/0x49 [vuln_msv]
> 
> Convert the driver to brightness_set_blocking and return the transport
> status to the LED core.
> 
> Fixes: 38433639af91 ("leds: leds-menf21bmc: Introduce MEN 14F021P00 BMC LED driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Runyu Xiao <runyu.xiao@seu.edu.cn>
> ---
> Notes:
>   - Validated with a grounded Lockdep PoC that preserves the
>     menf21bmc_led_probe() brightness_set registration and the
>     led_trigger_event_atomic() -> menf21bmc_led_set() ->
>     mutex_lock(&led_lock) path.
>   - checkpatch.pl --strict: clean.
>   - Not tested on MEN 14F021P00 BMC hardware.
> 
>  drivers/leds/leds-menf21bmc.c | 18 ++++++++++++------
>  1 file changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/leds/leds-menf21bmc.c b/drivers/leds/leds-menf21bmc.c
> index 6b1b47160602..da476fe6ed2c 100644
> --- a/drivers/leds/leds-menf21bmc.c
> +++ b/drivers/leds/leds-menf21bmc.c
> @@ -49,28 +49,33 @@ static struct menf21bmc_led leds[] = {
>  
>  static DEFINE_MUTEX(led_lock);
>  
> -static void
> -menf21bmc_led_set(struct led_classdev *led_cdev, enum led_brightness value)
> +static int
> +menf21bmc_led_set_blocking(struct led_classdev *led_cdev,
> +			   enum led_brightness value)
>  {
>  	int led_val;
> +	int ret = 0;
>  	struct menf21bmc_led *led = container_of(led_cdev,
>  					struct menf21bmc_led, cdev);
>  
>  	mutex_lock(&led_lock);
>  	led_val = i2c_smbus_read_byte_data(led->i2c_client,
>  					   BMC_CMD_LED_GET_SET);
> -	if (led_val < 0)
> +	if (led_val < 0) {
> +		ret = led_val;
>  		goto err_out;
> +	}
>  
>  	if (value == LED_OFF)
>  		led_val &= ~led->led_bit;
>  	else
>  		led_val |= led->led_bit;
>  
> -	i2c_smbus_write_byte_data(led->i2c_client,
> -				  BMC_CMD_LED_GET_SET, led_val);
> +	ret = i2c_smbus_write_byte_data(led->i2c_client,
> +					BMC_CMD_LED_GET_SET, led_val);
>  err_out:
>  	mutex_unlock(&led_lock);
> +	return ret;
>  }
>  
>  static int menf21bmc_led_probe(struct platform_device *pdev)
> @@ -81,7 +86,8 @@ static int menf21bmc_led_probe(struct platform_device *pdev)
>  
>  	for (i = 0; i < ARRAY_SIZE(leds); i++) {
>  		leds[i].cdev.name = leds[i].name;
> -		leds[i].cdev.brightness_set = menf21bmc_led_set;
> +		leds[i].cdev.brightness_set_blocking =
> +			menf21bmc_led_set_blocking;
>  		leds[i].i2c_client = i2c_client;
>  		ret = devm_led_classdev_register(&pdev->dev, &leds[i].cdev);
>  		if (ret < 0) {
> -- 
> 2.34.1

-- 
Lee Jones