Skip to content

Commit 17c13c7

Browse files
mzanderspavelmachek
authored andcommitted
leds: lp5523: fix out-of-bounds bug in lp5523_selftest()
When not all LED channels of the led chip are configured, the sysfs selftest functionality gives erroneous results and tries to test all channels of the chip. There is a potential for LED overcurrent conditions since the test current will be set to values from out-of-bound regions. It is wrong to use pdata->led_config[i].led_current to skip absent channels as led_config[] only contains the configured LED channels. Instead of iterating over all the physical channels of the device, loop over the available LED configurations and use led->chan_nr to access the correct i2c registers. Keep the zero-check for the LED current as existing users might depend on this to disable a channel. Reported-by: Arne Staessen <[email protected]> Signed-off-by: Maarten Zanders <[email protected]> Signed-off-by: Pavel Machek <[email protected]>
1 parent 5f52a8b commit 17c13c7

File tree

1 file changed

+15
-12
lines changed

1 file changed

+15
-12
lines changed

drivers/leds/leds-lp5523.c

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -581,8 +581,8 @@ static ssize_t lp5523_selftest(struct device *dev,
581581
struct lp55xx_led *led = i2c_get_clientdata(to_i2c_client(dev));
582582
struct lp55xx_chip *chip = led->chip;
583583
struct lp55xx_platform_data *pdata = chip->pdata;
584-
int i, ret, pos = 0;
585-
u8 status, adc, vdd;
584+
int ret, pos = 0;
585+
u8 status, adc, vdd, i;
586586

587587
mutex_lock(&chip->lock);
588588

@@ -612,41 +612,44 @@ static ssize_t lp5523_selftest(struct device *dev,
612612

613613
vdd--; /* There may be some fluctuation in measurement */
614614

615-
for (i = 0; i < LP5523_MAX_LEDS; i++) {
616-
/* Skip non-existing channels */
615+
for (i = 0; i < pdata->num_channels; i++) {
616+
/* Skip disabled channels */
617617
if (pdata->led_config[i].led_current == 0)
618618
continue;
619619

620620
/* Set default current */
621-
lp55xx_write(chip, LP5523_REG_LED_CURRENT_BASE + i,
621+
lp55xx_write(chip, LP5523_REG_LED_CURRENT_BASE + led->chan_nr,
622622
pdata->led_config[i].led_current);
623623

624-
lp55xx_write(chip, LP5523_REG_LED_PWM_BASE + i, 0xff);
624+
lp55xx_write(chip, LP5523_REG_LED_PWM_BASE + led->chan_nr,
625+
0xff);
625626
/* let current stabilize 2 - 4ms before measurements start */
626627
usleep_range(2000, 4000);
627628
lp55xx_write(chip, LP5523_REG_LED_TEST_CTRL,
628-
LP5523_EN_LEDTEST | i);
629+
LP5523_EN_LEDTEST | led->chan_nr);
629630
/* ADC conversion time is 2.7 ms typically */
630631
usleep_range(3000, 6000);
631632
ret = lp55xx_read(chip, LP5523_REG_STATUS, &status);
632633
if (ret < 0)
633634
goto fail;
634635

635636
if (!(status & LP5523_LEDTEST_DONE))
636-
usleep_range(3000, 6000);/* Was not ready. Wait. */
637+
usleep_range(3000, 6000); /* Was not ready. Wait. */
637638

638639
ret = lp55xx_read(chip, LP5523_REG_LED_TEST_ADC, &adc);
639640
if (ret < 0)
640641
goto fail;
641642

642643
if (adc >= vdd || adc < LP5523_ADC_SHORTCIRC_LIM)
643-
pos += sprintf(buf + pos, "LED %d FAIL\n", i);
644+
pos += sprintf(buf + pos, "LED %d FAIL\n",
645+
led->chan_nr);
644646

645-
lp55xx_write(chip, LP5523_REG_LED_PWM_BASE + i, 0x00);
647+
lp55xx_write(chip, LP5523_REG_LED_PWM_BASE + led->chan_nr,
648+
0x00);
646649

647650
/* Restore current */
648-
lp55xx_write(chip, LP5523_REG_LED_CURRENT_BASE + i,
649-
led->led_current);
651+
lp55xx_write(chip, LP5523_REG_LED_CURRENT_BASE + led->chan_nr,
652+
led->led_current);
650653
led++;
651654
}
652655
if (pos == 0)

0 commit comments

Comments
 (0)