From: Hans de Goede <hdegoede@redhat.com>
To: Pavel Machek <pavel@ucw.cz>, Lee Jones <lee@kernel.org>
Cc: Hans de Goede <hdegoede@redhat.com>, Kate Hsuan <hpa@redhat.com>,
linux-leds@vger.kernel.org
Subject: [PATCH] leds: trigger: Call synchronize_rcu() before calling trig->activate()
Date: Tue, 7 May 2024 11:51:11 +0200 [thread overview]
Message-ID: <20240507095111.27109-1-hdegoede@redhat.com> (raw)
Some triggers call led_trigger_event() from their activate() callback
to initialize the brightness of the LED for which the trigger is being
activated.
In order for the LEDs initial state to be set correctly this requires that
the led_trigger_event() call uses the new version of trigger->led_cdevs,
which has the new LED.
AFAICT led_trigger_event() will always use the new version when it is
running on the same CPU as where the list_add_tail_rcu() call was made,
which is why the missing synchronize_rcu() has not lead to bug reports.
But if activate() is pre-empted, sleeps or uses a worker then
the led_trigger_event() call may run on another CPU which may still use
the old trigger->led_cdevs list.
Add a synchronize_rcu() call to ensure that any led_trigger_event() calls
done from activate() always use the new list.
Triggers using led_trigger_event() from their activate() callback are:
net/bluetooth/leds.c, net/rfkill/core.c and drivers/tty/vt/keyboard.c.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/leds/led-triggers.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/leds/led-triggers.c b/drivers/leds/led-triggers.c
index 6d535a7fd075..018fe1eed4db 100644
--- a/drivers/leds/led-triggers.c
+++ b/drivers/leds/led-triggers.c
@@ -194,6 +194,13 @@ int led_trigger_set(struct led_classdev *led_cdev, struct led_trigger *trig)
spin_unlock(&trig->leddev_list_lock);
led_cdev->trigger = trig;
+ /*
+ * Some activate() calls use led_trigger_event() to initialize
+ * the brightness of the LED for which the trigger is being set.
+ * Ensure the led_cdev is visible on trig->led_cdevs for this.
+ */
+ synchronize_rcu();
+
if (trig->activate)
ret = trig->activate(led_cdev);
else
--
2.44.0
next reply other threads:[~2024-05-07 9:51 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-07 9:51 Hans de Goede [this message]
2024-05-31 10:46 ` [PATCH] leds: trigger: Call synchronize_rcu() before calling trig->activate() Lee Jones
2024-05-31 12:40 ` (subset) " Lee Jones
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240507095111.27109-1-hdegoede@redhat.com \
--to=hdegoede@redhat.com \
--cc=hpa@redhat.com \
--cc=lee@kernel.org \
--cc=linux-leds@vger.kernel.org \
--cc=pavel@ucw.cz \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).