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

[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

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

[Lee Jones]

On Wed, 28 Oct 2026, Danish Khateeb wrote:

> 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;

This is replaced in patch 2.  I'm not sure I see the point.

Why not do everything at once?

>  	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
>

-- 
Lee Jones

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

[Andrew Davis]

On 11/4/26 12:22 PM, Lee Jones wrote:
> On Wed, 28 Oct 2026, Danish Khateeb wrote:
> 
>> 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;
> 
> This is replaced in patch 2.  I'm not sure I see the point.
> 
> Why not do everything at once?
> 

I see a v4 combining these just got posted before I could make
a reply here, but going to reply here anyway.

The two patches do different logical things, the first restores
the order of unregistering the LED before cancelling the work,
which fixes 1b5c2fa7081c. The second is a change in behavior, as
now we flush the work, not just cancel it as had previously always
been the case.

Both changes are correct, so combining is probably fine, but it
does slightly obscure the cancel -> flush behavior change part.

Andrew

>>   	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
>>
>