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