* [PATCH v3 1/2] leds: tca6507: Fix use-after-free on unbind [not found] <20261028174348.64014-1-danishkhateeb03@gmail.com> @ 2026-10-28 17:43 ` Danish Khateeb 2026-11-04 18:22 ` Lee Jones 0 siblings, 1 reply; 3+ messages in thread From: Danish Khateeb @ 2026-10-28 17:43 UTC (permalink / raw) To: Lee Jones, Pavel Machek Cc: Andrew Davis, linux-leds, linux-kernel, Danish Khateeb, stable 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 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v3 1/2] leds: tca6507: Fix use-after-free on unbind 2026-10-28 17:43 ` [PATCH v3 1/2] leds: tca6507: Fix use-after-free on unbind Danish Khateeb @ 2026-11-04 18:22 ` Lee Jones 2026-11-04 19:46 ` Andrew Davis 0 siblings, 1 reply; 3+ messages in thread From: Lee Jones @ 2026-11-04 18:22 UTC (permalink / raw) To: Danish Khateeb Cc: Pavel Machek, Andrew Davis, linux-leds, linux-kernel, stable 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 ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v3 1/2] leds: tca6507: Fix use-after-free on unbind 2026-11-04 18:22 ` Lee Jones @ 2026-11-04 19:46 ` Andrew Davis 0 siblings, 0 replies; 3+ messages in thread From: Andrew Davis @ 2026-11-04 19:46 UTC (permalink / raw) To: Lee Jones, Danish Khateeb; +Cc: Pavel Machek, linux-leds, linux-kernel, stable 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 >> > ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-11-04 19:46 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20261028174348.64014-1-danishkhateeb03@gmail.com>
2026-10-28 17:43 ` [PATCH v3 1/2] leds: tca6507: Fix use-after-free on unbind Danish Khateeb
2026-11-04 18:22 ` Lee Jones
2026-11-04 19:46 ` Andrew Davis
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox