RE: [PATCH] leds: pca9532: check i2c_smbus_read_byte_data() return value
Bough Chen <[email protected]>
| Newsgroups | dev.linux.lists.imx,org.kernel.vger.linux-kernel,org.kernel.vger.linux-leds |
|---|---|
| Message-ID | <DU2PR04MB8677182F2CC98B6AAD09F25990CC2@DU2PR04MB8677.eurprd04.prod.outlook.com> |
> -----Original Message----- > From: Bough Chen (OSS) <[email protected]> > Sent: 2026年7月27日 17:58 > To: Riku Voipio <[email protected]>; Lee Jones <[email protected]>; Pavel > Machek <[email protected]> > Cc: [email protected]; [email protected]; > [email protected]; Bough Chen <[email protected]> > Subject: [PATCH] leds: pca9532: check i2c_smbus_read_byte_data() return > value > > From: Haibo Chen <[email protected]> > > Coverity report INTEGER_OVERFLOW (tainted_data_return -> cast_underflow > -> overflow) in pca9532_getled(). > pca9532_getled() stored the return value of i2c_smbus_read_byte_data() into > a char and used it directly to compute the LED state without checking for an > error. i2c_smbus_read_byte_data() returns a negative error code on failure. > On an I2C read error the negative value was assigned to an unsigned type, > triggering an integer underflow/overflow, and a meaningless LED state was > returned to the caller. > > Read the value into an int, check for the error, and fall back to PCA9532_OFF > while emitting a warning when the read fails. Sorry, I forget to add the fix tag: Fixes: 28c5fe99016d ("leds: pca9532: Extend pca9532 device tree support") Will add it into commit log if need a V2. Regards Haibo Chen > > Assisted-by: VeroCoder:vero-1 [cli] > Signed-off-by: Haibo Chen <[email protected]> > --- > drivers/leds/leds-pca9532.c | 9 ++++++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/drivers/leds/leds-pca9532.c b/drivers/leds/leds-pca9532.c index > 2d37e00e459dee9f0313830f4852b01f40977311..e8ee17b8bd547a92372a8 > e74a6781b95ae831fe2 100644 > --- a/drivers/leds/leds-pca9532.c > +++ b/drivers/leds/leds-pca9532.c > @@ -298,12 +298,15 @@ static enum pca9532_state pca9532_getled(struct > pca9532_led *led) > struct i2c_client *client = led->client; > struct pca9532_data *data = i2c_get_clientdata(client); > u8 maxleds = data->chip_info->num_leds; > - char reg; > - enum pca9532_state ret; > + int reg; > + enum pca9532_state ret = PCA9532_OFF; > > mutex_lock(&data->update_lock); > reg = i2c_smbus_read_byte_data(client, LED_REG(maxleds, led->id)); > - ret = (reg & LED_MASK(led->id)) >> LED_SHIFT(led->id); > + if (reg >= 0) > + ret = (reg & LED_MASK(led->id)) >> LED_SHIFT(led->id); > + else > + dev_warn(&client->dev, "failed to read LED register: %d\n", reg); > mutex_unlock(&data->update_lock); > return ret; > } > > --- > base-commit: c5e32e86ca02b003f86e095d379b38148999293d > change-id: 20260727-led-fix-28f085e12feb > > Best regards, > -- > Haibo Chen <[email protected]>