[PATCH v2 0/4] leds: pca9532: fix unchecked I2C reads and unbind use-after-free

[PATCH v2 0/4] leds: pca9532: fix unchecked I2C reads and unbind use-after-free

[haibo.chen]

This series hardens the pca9532 LED driver against unchecked
i2c_smbus_read_byte_data() return values and fixes a use-after-free that
can happen on device unbind.

Patch 1 (the only patch in v1) checks the return value of
i2c_smbus_read_byte_data() in pca9532_getled(), fixing a Coverity
INTEGER_OVERFLOW report where an I2C error code was cast into a char and
used to compute a bogus LED state.

While reviewing v1, Sashiko AI review pointed out three additional
pre-existing issues in the same driver. Patches 2-4 address them:

- Patch 2: pca9532_setled() has the identical unchecked-read flaw, but it
  is worse: one register controls four LEDs, so writing the truncated
  error code back corrupts the state of the other three LEDs sharing the
  register. The return value is now checked and propagated to callers.

- Patch 3: pca9532_gpio_get_value() stores the read result in an unsigned
  char, so an I2C error is truncated and returned as a valid GPIO level,
  silently handing corrupt data to kernel/userspace consumers. The read
  is now stored in an int and the error propagated.

- Patch 4: on unbind, the input device allocated with
  devm_input_allocate_device() stays registered until devres cleanup runs
  after .remove(). pca9532_destroy_devices() only cancels the work, so an
  EV_SND event arriving after cancel_work_sync() can re-queue the work and
  dereference the freed driver data. The input device is now explicitly
  unregistered before the work is cancelled.

All patches build cleanly with CONFIG_LEDS_PCA9532 and
CONFIG_LEDS_PCA9532_GPIO enabled and pass checkpatch.

Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
---
Changes in v2:
- Patch 1: reworded the commit log and added an Assisted-by trailer.
  No functional change.
- New Patch 2: check pca9532_setled() I2C read return value (pre-existing
  issue reported by Sashiko AI review of v1). Cc: stable.
- New Patch 3: check pca9532_gpio_get_value() I2C read return value
  (pre-existing issue reported by Sashiko AI review of v1). Cc: stable.
- New Patch 4: fix use-after-free on unbind with the N2100 beeper
  (pre-existing issue reported by Sashiko AI review of v1). Cc: stable.
- Link to v1: https://lore.kernel.org/r/20260727-led-fix-v1-1-d635c5037109@nxp.com

---
Haibo Chen (4):
      leds: pca9532: check i2c_smbus_read_byte_data() return value
      leds: pca9532: check return value in pca9532_setled()
      leds: pca9532: check return value in pca9532_gpio_get_value()
      leds: pca9532: fix use-after-free on unbind with N2100 beeper

 drivers/leds/leds-pca9532.c | 47 ++++++++++++++++++++++++++++++---------------
 1 file changed, 32 insertions(+), 15 deletions(-)
---
base-commit: c5e32e86ca02b003f86e095d379b38148999293d
change-id: 20260727-led-fix-28f085e12feb

Best regards,
-- 
Haibo Chen <haibo.chen@nxp.com>

[PATCH v2 1/4] leds: pca9532: check i2c_smbus_read_byte_data() return value

[haibo.chen]

From: Haibo Chen <haibo.chen@nxp.com>

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.

Fixes: 28c5fe99016d ("leds: pca9532: Extend pca9532 device tree support")
Assisted-by: VeroCoder:claude-sonnet-4
Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
---
 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..e8ee17b8bd547a92372a8e74a6781b95ae831fe2 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;
 }

-- 
2.34.1

[PATCH v2 2/4] leds: pca9532: check return value in pca9532_setled()

[haibo.chen]

From: Haibo Chen <haibo.chen@nxp.com>

pca9532_setled() reads the LED register with i2c_smbus_read_byte_data()
but never checks its return value. On an I2C read failure the function
returns a negative error code (e.g. -EIO) which was silently truncated
into a signed char, then bitwise modified and written back to the
device.

Since one LED register controls four LEDs, writing this bogus value back
corrupts the state of the other three LEDs sharing the same register.

Fixes: e14fa82439d3 ("leds: Add pca9532 led driver")
Cc: stable@vger.kernel.org
Assisted-by: VeroCoder:claude-sonnet-4
Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
---
 drivers/leds/leds-pca9532.c | 25 ++++++++++++++-----------
 1 file changed, 14 insertions(+), 11 deletions(-)

diff --git a/drivers/leds/leds-pca9532.c b/drivers/leds/leds-pca9532.c
index e8ee17b8bd547a92372a8e74a6781b95ae831fe2..adfb5aa50e8e1f3ca0deb9261d8c9103903ef928 100644
--- a/drivers/leds/leds-pca9532.c
+++ b/drivers/leds/leds-pca9532.c
@@ -159,21 +159,29 @@ static int pca9532_setpwm(struct i2c_client *client, int pwm)
 }
 
 /* Set LED routing */
-static void pca9532_setled(struct pca9532_led *led)
+static int pca9532_setled(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;
+	int reg;
+	int ret = 0;
 
 	mutex_lock(&data->update_lock);
 	reg = i2c_smbus_read_byte_data(client, LED_REG(maxleds, led->id));
+	if (reg < 0) {
+		dev_warn(&client->dev, "failed to read LED register: %d\n", reg);
+		ret = reg;
+		goto out;
+	}
 	/* zero led bits */
 	reg = reg & ~LED_MASK(led->id);
 	/* set the new value */
 	reg = reg | (led->state << LED_SHIFT(led->id));
-	i2c_smbus_write_byte_data(client, LED_REG(maxleds, led->id), reg);
+	ret = i2c_smbus_write_byte_data(client, LED_REG(maxleds, led->id), reg);
+out:
 	mutex_unlock(&data->update_lock);
+	return ret;
 }
 
 static int pca9532_set_brightness(struct led_classdev *led_cdev,
@@ -196,8 +204,7 @@ static int pca9532_set_brightness(struct led_classdev *led_cdev,
 	}
 	if (led->state == PCA9532_PWM0)
 		pca9532_setpwm(led->client, PCA9532_PWM_ID_0);
-	pca9532_setled(led);
-	return err;
+	return pca9532_setled(led);
 }
 
 static int pca9532_update_hw_blink(struct pca9532_led *led,
@@ -257,9 +264,7 @@ static int pca9532_set_blink(struct led_classdev *led_cdev,
 	if (err)
 		return err;
 
-	pca9532_setled(led);
-
-	return 0;
+	return pca9532_setled(led);
 }
 
 static int pca9532_event(struct input_dev *dev, unsigned int type,
@@ -334,9 +339,7 @@ static int pca9532_gpio_set_value(struct gpio_chip *gc, unsigned int offset,
 	else
 		led->state = PCA9532_ON;
 
-	pca9532_setled(led);
-
-	return 0;
+	return pca9532_setled(led);
 }
 
 static int pca9532_gpio_get_value(struct gpio_chip *gc, unsigned offset)

-- 
2.34.1

[PATCH v2 3/4] leds: pca9532: check return value in pca9532_gpio_get_value()

[haibo.chen]

From: Haibo Chen <haibo.chen@nxp.com>

pca9532_gpio_get_value() reads the input register with
i2c_smbus_read_byte_data() but stores the result in an unsigned char and
never checks for failure. On an I2C read error the negative error code
(e.g. -EIO) is truncated to an unsigned byte, and a bit is extracted and
returned as a valid GPIO level. This masks hardware errors and lets
kernel or userspace consumers silently receive corrupt data.

The gpio_chip .get callback may return a negative error code, so store
the read result in an int and propagate the error on failure.

Fixes: 3c1ab50d0a31 ("drivers/leds/leds-pca9532.c: add gpio capability")
Cc: stable@vger.kernel.org
Assisted-by: VeroCoder:claude-sonnet-4
Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
---
 drivers/leds/leds-pca9532.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/leds/leds-pca9532.c b/drivers/leds/leds-pca9532.c
index adfb5aa50e8e1f3ca0deb9261d8c9103903ef928..cee398d8275b8953b5dbdb741c36161f34bba029 100644
--- a/drivers/leds/leds-pca9532.c
+++ b/drivers/leds/leds-pca9532.c
@@ -345,9 +345,14 @@ static int pca9532_gpio_set_value(struct gpio_chip *gc, unsigned int offset,
 static int pca9532_gpio_get_value(struct gpio_chip *gc, unsigned offset)
 {
 	struct pca9532_data *data = gpiochip_get_data(gc);
-	unsigned char reg;
+	int reg;
 
 	reg = i2c_smbus_read_byte_data(data->client, PCA9532_REG_INPUT(offset));
+	if (reg < 0) {
+		dev_warn(&data->client->dev,
+			 "failed to read input register: %d\n", reg);
+		return reg;
+	}
 
 	return !!(reg & (1 << (offset % 8)));
 }

-- 
2.34.1

[PATCH v2 4/4] leds: pca9532: fix use-after-free on unbind with N2100 beeper

[haibo.chen]

From: Haibo Chen <haibo.chen@nxp.com>

The input device for the N2100 beeper is allocated with
devm_input_allocate_device(), so it stays registered until devres
cleanup runs after .remove() returns. pca9532_destroy_devices() only
calls cancel_work_sync() and clears data->idev, but leaves the input
device registered.

In the window between cancel_work_sync() returning and devres
unregistering the input device, a userspace EV_SND event can reach
pca9532_event(), which calls schedule_work() and re-queues data->work
after it was cancelled. Once devres frees the devm-allocated driver
data, the work callback dereferences freed memory, resulting in a
use-after-free.

Explicitly unregister the input device before cancelling the work so
that no new events can be delivered after the work has been cancelled.

Fixes: 8614fb46637a ("leds: pca9532: Convert to devm_input_allocate_device()")
Cc: stable@vger.kernel.org
Assisted-by: VeroCoder:claude-sonnet-4
Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
---
 drivers/leds/leds-pca9532.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/leds/leds-pca9532.c b/drivers/leds/leds-pca9532.c
index cee398d8275b8953b5dbdb741c36161f34bba029..eeb91b7466419325191200f6cd9a4dc81f8ee20c 100644
--- a/drivers/leds/leds-pca9532.c
+++ b/drivers/leds/leds-pca9532.c
@@ -385,6 +385,12 @@ static void pca9532_destroy_devices(struct pca9532_data *data, int n_devs)
 			break;
 		case PCA9532_TYPE_N2100_BEEP:
 			if (data->idev != NULL) {
+				/*
+				 * Unregister the input device first so that no
+				 * new EV_SND events can re-queue the work after
+				 * it has been cancelled.
+				 */
+				input_unregister_device(data->idev);
 				cancel_work_sync(&data->work);
 				data->idev = NULL;
 			}

-- 
2.34.1