Re: [PATCH AUTOSEL 6.18] leds: tps6131x: Increase overvoltage protection threshold to 6V

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Missing cleanup and inverted teardown order for `torch_refresh_work` causes use-after-free.
- [High] The flash strobe is immediately cancelled when turned on due to a missing return statement.
- [High] `led_cdev->max_brightness` is set to milliamperes instead of steps, breaking the standard sysfs brightness scaling interface.
--

commit d25747de0af565ce7fa5d6951483e0ef6ee5bb6d
Author: Matthias Fend <matthias.fend@emfend.at>

leds: tps6131x: Increase overvoltage protection threshold to 6V

This commit increases the overvoltage protection threshold in the tps6131x
driver to 6V. It adjusts the register initialization value to prevent spurious
overvoltage detection on valid 5V hardware setups.

Link: https://patch.msgid.link/20260527-leds-tps6131x-ovp-v1-1-1ac70d03c9eb@emfend.at

> diff --git a/drivers/leds/flash/leds-tps6131x.c b/drivers/leds/flash/leds-tps6131x.c
> index 6f4d4fd553610..ad96455b3b02a 100644
> --- a/drivers/leds/flash/leds-tps6131x.c
> +++ b/drivers/leds/flash/leds-tps6131x.c
> @@ -277,7 +277,7 @@ static int tps6131x_init_chip(struct tps6131x *tps6131x)
>  	if (ret)
>  		return ret;
>  
> -	val = TPS6131X_REG_6_ENTS;
> +	val = TPS6131X_REG_6_ENTS | (TPS6131X_OV_4950MV << TPS6131X_REG_6_OV_SHIFT);
>  
>  	ret = regmap_write(tps6131x->regmap, TPS6131X_REG_6, val);
>  	if (ret)

[ ... ]

> static void tps6131x_remove(struct i2c_client *client)
> {
> 	struct tps6131x *tps6131x = i2c_get_clientdata(client);
> 
> 	v4l2_flash_release(tps6131x->v4l2_flash);
> 
> 	cancel_delayed_work_sync(&tps6131x->torch_refresh_work);
> }

[Severity: High]
This isn't a bug introduced by this patch, but doesn't this teardown order
leave a use-after-free vulnerability in tps6131x_remove()?

Because devm_led_classdev_flash_register_ext() uses devres, the LED device
remains registered when tps6131x_remove() returns. If a user writes to the
LED brightness sysfs attribute during this window, tps6131x_brightness_set()
can reschedule torch_refresh_work. 

The subsequent devres teardown will free the tps6131x struct while the delayed
work timer is still active.

Additionally, in tps6131x_probe():

> 	ret = tps6131x_v4l2_setup(tps6131x);
> 	if (ret)
> 		return dev_err_probe(&client->dev, ret, "Failed to setup v4l2 flash\n");

[Severity: High]
This is a pre-existing issue, but is the timer leaked in this error path?

If tps6131x_v4l2_setup() fails, the probe exits without canceling the delayed
work. Since the LED class device was already registered and could have been
accessed, the timer might still be active.

[ ... ]

> static int tps6131x_strobe_set(struct led_classdev_flash *fled_cdev, bool state)
> {
[ ... ]
> 	if (state) {
> 		ret = regmap_update_bits_base(tps6131x->regmap, TPS6131X_REG_3, TPS6131X_REG_3_SFT,
> 					      TPS6131X_REG_3_SFT, NULL, false, true);
> 		if (ret)
> 			return ret;
> 	}
> 
> 	ret = regmap_update_bits_base(tps6131x->regmap, TPS6131X_REG_3, TPS6131X_REG_3_SFT, 0, NULL,
> 				      false, true);

[Severity: High]
This is a pre-existing issue, but will this immediately terminate the flash
pulse?

If state is true, the driver writes a 1 to TPS6131X_REG_3_SFT, but without
a return statement inside the if-block, execution falls through and writes a
0 to TPS6131X_REG_3_SFT. This appears to break the hardware strobe
functionality by immediately turning it off.

[ ... ]

> static int tps6131x_led_class_setup(struct tps6131x *tps6131x)
> {
[ ... ]
> 	led_cdev->brightness_set_blocking = tps6131x_brightness_set;
> 	led_cdev->max_brightness = tps6131x->max_torch_current_ma;

[Severity: High]
This isn't a bug introduced by this patch, but is max_brightness incorrectly
configured with milliamperes rather than step counts?

In tps6131x_brightness_set(), the brightness parameter is interpreted directly
as the number of steps:

> static int tps6131x_brightness_set(struct led_classdev *cdev, enum led_brightness brightness)
> {
[ ... ]
> 	steps_remaining = (brightness * tps6131x->step_torch_current_ma) / TPS6131X_TORCH_STEP_I_MA;

If max_brightness is exposed as milliamperes (e.g., 175), writing values up
to 175 via sysfs will be processed as 175 steps and clamped to the hardware
maximum. This saturates the computation and locks the output to 100% brightness
for almost the entire exposed sysfs range.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831133314.4125787-567-sashal@kernel.org?part=1