diff mbox series

leds: trigger: Unregister sysfs attributes before calling deactivate()

Message ID 20240504162533.76780-1-hdegoede@redhat.com
State New
Headers show
Series leds: trigger: Unregister sysfs attributes before calling deactivate() | expand

Commit Message

Hans de Goede May 4, 2024, 4:25 p.m. UTC
Triggers which have trigger specific sysfs attributes typically store
related data in trigger-data allocated by the activate() callback and
freed by the deactivate() callback.

Calling device_remove_groups() after calling deactivate() leaves a window
where the sysfs attributes show/store functions could be called after
deactivation and then operate on the just freed trigger-data.

Move the device_remove_groups() call to before deactivate() to close
this race window.

This also makes the deactivation path properly do things in reverse order
of the activation path which calls the activate() callback before calling
device_add_groups().

Fixes: a7e7a3156300 ("leds: triggers: add device attribute support")
Cc: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/leds/led-triggers.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Uwe Kleine-König May 6, 2024, 6:56 a.m. UTC | #1
On Sat, May 04, 2024 at 06:25:33PM +0200, Hans de Goede wrote:
> Triggers which have trigger specific sysfs attributes typically store
> related data in trigger-data allocated by the activate() callback and
> freed by the deactivate() callback.
> 
> Calling device_remove_groups() after calling deactivate() leaves a window
> where the sysfs attributes show/store functions could be called after
> deactivation and then operate on the just freed trigger-data.
> 
> Move the device_remove_groups() call to before deactivate() to close
> this race window.
> 
> This also makes the deactivation path properly do things in reverse order
> of the activation path which calls the activate() callback before calling
> device_add_groups().
> 
> Fixes: a7e7a3156300 ("leds: triggers: add device attribute support")
> Cc: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Looks reasonable

Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Thanks
Uwe
Lee Jones May 31, 2024, 10:30 a.m. UTC | #2
On Sat, 04 May 2024 18:25:33 +0200, Hans de Goede wrote:
> Triggers which have trigger specific sysfs attributes typically store
> related data in trigger-data allocated by the activate() callback and
> freed by the deactivate() callback.
> 
> Calling device_remove_groups() after calling deactivate() leaves a window
> where the sysfs attributes show/store functions could be called after
> deactivation and then operate on the just freed trigger-data.
> 
> [...]

Applied, thanks!

[1/1] leds: trigger: Unregister sysfs attributes before calling deactivate()
      commit: 1a71c0768c716f6aecc633246b55e7f4cf9c6c82

--
Lee Jones [李琼斯]
diff mbox series

Patch

diff --git a/drivers/leds/led-triggers.c b/drivers/leds/led-triggers.c
index d72c42f38bd4..6d535a7fd075 100644
--- a/drivers/leds/led-triggers.c
+++ b/drivers/leds/led-triggers.c
@@ -179,9 +179,9 @@  int led_trigger_set(struct led_classdev *led_cdev, struct led_trigger *trig)
 
 		cancel_work_sync(&led_cdev->set_brightness_work);
 		led_stop_software_blink(led_cdev);
+		device_remove_groups(led_cdev->dev, led_cdev->trigger->groups);
 		if (led_cdev->trigger->deactivate)
 			led_cdev->trigger->deactivate(led_cdev);
-		device_remove_groups(led_cdev->dev, led_cdev->trigger->groups);
 		led_cdev->trigger = NULL;
 		led_cdev->trigger_data = NULL;
 		led_cdev->activated = false;