[PATCH v3 0/2] leds: tca6507: Fix use-after-free on unbind and turn the LEDs off

[PATCH v3 0/2] leds: tca6507: Fix use-after-free on unbind and turn the LEDs off

[Danish Khateeb]

Patch 1 is v2 unchanged apart from the rebase. It sets the work up with
devm_work_autocancel() so devres cancels it after the LEDs are gone.

Sashiko pointed out that the "LED off" write queued by unregistering the
LEDs then gets cancelled, so an LED that was on may stay on after unbind.
That was already the case before 1b5c2fa7081c. Patch 2 fixes it by
flushing the work instead of cancelling it. It is not marked for stable.

Changes in v3:
- Add patch 2 (Lee)
- Rebase on for-leds-next
- Link to v2: https://lore.kernel.org/all/20261015144033.56519-1-danishkhateeb03@gmail.com/

Tested in QEMU on a next-20260930 KASAN kernel, with the driver built as
a module and a test-only module that adds a fake I2C adapter, logs the
register writes and adds a software node for a TCA6507 with two LEDs.
With both LEDs on, each of 20 unbinds was checked for a final write
clearing both LEDs: with patch 1 alone it was missing in 7 of 20, with
patch 2 in none. There were no KASAN reports with either, over 20
bind/unbind cycles and an unload. A W=1 build with GPIOLIB is clean.

An LLM was used to help with this series.

Danish Khateeb (2):
  leds: tca6507: Fix use-after-free on unbind
  leds: tca6507: Turn the LEDs off on unbind

 drivers/leds/leds-tca6507.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)


base-commit: 59cf91cf87abb67a49b1bb7b9d3b06486abde8cf
-- 
2.56.0

[PATCH v3 1/2] leds: tca6507: Fix use-after-free on unbind

[Danish Khateeb]

Since the LEDs are registered with devm_led_classdev_register(), they
are unregistered only after tca6507_remove() has cancelled the work.
Unregistering an LED that is on turns it off, which queues the work
again from led_assign(), and the work then runs after devres has freed
the chip:

  BUG: KASAN: slab-use-after-free in assign_work+0x414/0x5b0
  Read of size 8 at addr ffff888010d5c0c0 by task kworker/1:2/65
  ...
  Last potentially related work creation:
   ...
   queue_work_on+0xb6/0xc0
   led_assign+0x223/0x340 [leds_tca6507]
   led_classdev_unregister+0x26b/0x340
   release_nodes+0xb3/0x140
   devres_release_group+0x247/0x470
   i2c_device_remove+0xbf/0x1b0

Set up the work with devm_work_autocancel() before the LEDs are
registered, so that devres cancels it after they are gone, and drop
tca6507_remove().

Fixes: 1b5c2fa7081c ("leds: tca6507: Use devm_led_classdev_register() to simplify remove path")
Cc: stable@vger.kernel.org
Acked-by: Andrew Davis <afd@ti.com>
Signed-off-by: Danish Khateeb <danishkhateeb03@gmail.com>
---
 drivers/leds/leds-tca6507.c | 15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/drivers/leds/leds-tca6507.c b/drivers/leds/leds-tca6507.c
index 9afe2722986c..0f987e551f03 100644
--- a/drivers/leds/leds-tca6507.c
+++ b/drivers/leds/leds-tca6507.c
@@ -71,6 +71,7 @@
  * not recognised when matching.
  */
 
+#include <linux/devm-helpers.h>
 #include <linux/module.h>
 #include <linux/slab.h>
 #include <linux/leds.h>
@@ -735,9 +736,11 @@ static int tca6507_probe(struct i2c_client *client)
 		return -ENOMEM;
 
 	tca->client = client;
-	INIT_WORK(&tca->work, tca6507_work);
 	spin_lock_init(&tca->lock);
-	i2c_set_clientdata(client, tca);
+
+	err = devm_work_autocancel(dev, &tca->work, tca6507_work);
+	if (err)
+		return err;
 
 	for (i = 0; i < NUM_LEDS; i++) {
 		struct tca6507_led *l = tca->leds + i;
@@ -766,20 +769,12 @@ static int tca6507_probe(struct i2c_client *client)
 	return 0;
 }
 
-static void tca6507_remove(struct i2c_client *client)
-{
-	struct tca6507_chip *tca = i2c_get_clientdata(client);
-
-	cancel_work_sync(&tca->work);
-}
-
 static struct i2c_driver tca6507_driver = {
 	.driver   = {
 		.name    = "leds-tca6507",
 		.of_match_table = of_match_ptr(of_tca6507_leds_match),
 	},
 	.probe    = tca6507_probe,
-	.remove   = tca6507_remove,
 	.id_table = tca6507_id,
 };
 
-- 
2.56.0

[PATCH v3 2/2] leds: tca6507: Turn the LEDs off on unbind

[Danish Khateeb]

Unregistering an LED that is on turns it off, and the driver does that
from its work, which is queued from led_assign(). On unbind the work is
then cancelled by devres, so if it has not run yet the LED stays on after
the driver is gone.

Flush the work instead of cancelling it. By the time the devres action
runs, the LEDs and the GPIO chip have been unregistered, and the work
does not queue itself, so nothing can queue it again.

Signed-off-by: Danish Khateeb <danishkhateeb03@gmail.com>
---
 drivers/leds/leds-tca6507.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/leds/leds-tca6507.c b/drivers/leds/leds-tca6507.c
index 0f987e551f03..ffad476af625 100644
--- a/drivers/leds/leds-tca6507.c
+++ b/drivers/leds/leds-tca6507.c
@@ -71,7 +71,6 @@
  * not recognised when matching.
  */
 
-#include <linux/devm-helpers.h>
 #include <linux/module.h>
 #include <linux/slab.h>
 #include <linux/leds.h>
@@ -712,6 +711,11 @@ static const struct of_device_id __maybe_unused of_tca6507_leds_match[] = {
 };
 MODULE_DEVICE_TABLE(of, of_tca6507_leds_match);
 
+static void tca6507_flush_work(void *work)
+{
+	flush_work(work);
+}
+
 static int tca6507_probe(struct i2c_client *client)
 {
 	struct device *dev = &client->dev;
@@ -738,7 +742,8 @@ static int tca6507_probe(struct i2c_client *client)
 	tca->client = client;
 	spin_lock_init(&tca->lock);
 
-	err = devm_work_autocancel(dev, &tca->work, tca6507_work);
+	INIT_WORK(&tca->work, tca6507_work);
+	err = devm_add_action_or_reset(dev, tca6507_flush_work, &tca->work);
 	if (err)
 		return err;
 
-- 
2.56.0

Re: [PATCH v3 1/2] leds: tca6507: Fix use-after-free on unbind

[sashiko-bot]

> Since the LEDs are registered with devm_led_classdev_register(), they
> are unregistered only after tca6507_remove() has cancelled the work.
> Unregistering an LED that is on turns it off, which queues the work
> again from led_assign(), and the work then runs after devres has freed
> the chip:
> 
>   BUG: KASAN: slab-use-after-free in assign_work+0x414/0x5b0
>   Read of size 8 at addr ffff888010d5c0c0 by task kworker/1:2/65
>   ...
>   Last potentially related work creation:
>    ...
>    queue_work_on+0xb6/0xc0
>    led_assign+0x223/0x340 [leds_tca6507]
>    led_classdev_unregister+0x26b/0x340
>    release_nodes+0xb3/0x140
>    devres_release_group+0x247/0x470
>    i2c_device_remove+0xbf/0x1b0
> 
> Set up the work with devm_work_autocancel() before the LEDs are
> registered, so that devres cancels it after they are gone, and drop
> tca6507_remove().
> 
> Fixes: 1b5c2fa7081c ("leds: tca6507: Use devm_led_classdev_register() to simplify remove path")
> Cc: stable@vger.kernel.org
> Acked-by: Andrew Davis <afd@ti.com>
> Signed-off-by: Danish Khateeb <danishkhateeb03@gmail.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261028174348.64014-1-danishkhateeb03@gmail.com?part=1

Re: [PATCH v3 2/2] leds: tca6507: Turn the LEDs off on unbind

[sashiko-bot]

> Unregistering an LED that is on turns it off, and the driver does that
> from its work, which is queued from led_assign(). On unbind the work is
> then cancelled by devres, so if it has not run yet the LED stays on after
> the driver is gone.
> 
> Flush the work instead of cancelling it. By the time the devres action
> runs, the LEDs and the GPIO chip have been unregistered, and the work
> does not queue itself, so nothing can queue it again.
> 
> Signed-off-by: Danish Khateeb <danishkhateeb03@gmail.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261028174348.64014-1-danishkhateeb03@gmail.com?part=2