[PATCH] ALSA: control: Synchronize LED mode accesses

[PATCH] ALSA: control: Synchronize LED mode accesses

[Sreeraj S Kurup]

mode_store() updates led->mode while holding
snd_ctl_led_mutex.

However, snd_ctl_led_set_state() and mode_show() read this
field without holding the same mutex.

This can cause a data race when userspace changes the mode
while another path reads it.

Read the mode under snd_ctl_led_mutex in
snd_ctl_led_set_state() and use the local value after
releasing the mutex.

Also protect the mode read in mode_show() with the mutex.

Signed-off-by: Sreeraj S Kurup <sreekuttan2156239@gmail.com>
---
 sound/core/control_led.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/sound/core/control_led.c b/sound/core/control_led.c
index 501387287ae6..044430b2f1a2 100644
--- a/sound/core/control_led.c
+++ b/sound/core/control_led.c
@@ -140,6 +140,7 @@ static void snd_ctl_led_set_state(struct snd_card *card, unsigned int access,
 {
 	struct snd_ctl_led *led;
 	struct snd_ctl_led_ctl *lctl;
+	enum snd_ctl_led_mode mode;
 	int route;
 	bool found;
 
@@ -168,8 +169,9 @@ static void snd_ctl_led_set_state(struct snd_card *card, unsigned int access,
 				UPDATE_ROUTE(route, snd_ctl_led_get(lctl));
 			}
 		}
+		mode = led->mode;
 	}
-	switch (led->mode) {
+	switch (mode) {
 	case MODE_OFF:		route = 1; break;
 	case MODE_ON:		route = 0; break;
 	case MODE_FOLLOW_ROUTE:	if (route >= 0) route ^= 1; break;
@@ -382,6 +384,8 @@ static ssize_t mode_show(struct device *dev,
 	struct snd_ctl_led *led = container_of(dev, struct snd_ctl_led, dev);
 	const char *str = NULL;
 
+	guard(mutex)(&snd_ctl_led_mutex);
+
 	switch (led->mode) {
 	case MODE_FOLLOW_MUTE:	str = "follow-mute"; break;
 	case MODE_FOLLOW_ROUTE:	str = "follow-route"; break;
-- 
2.55.0

[PATCH v2] ALSA: control: Synchronize LED mode accesses

[Sreeraj S Kurup]

mode_store() updates led->mode while holding
snd_ctl_led_mutex.

However, snd_ctl_led_set_state() and mode_show() read this
field without holding the same mutex.

This can cause a data race when userspace changes the mode
while another path reads it.

Read the mode under snd_ctl_led_mutex in
snd_ctl_led_set_state() and use the local value after
releasing the mutex.

Also protect the mode read in mode_show() with the mutex.

Fixes: cb17fe0045aa ("ALSA: control - add sysfs support to the LED trigger module")
Signed-off-by: Sreeraj S Kurup <sreekuttan2156239@gmail.com>
---
v2:
 - Add the Fixes trailer.

 sound/core/control_led.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/sound/core/control_led.c b/sound/core/control_led.c
index 501387287ae6..044430b2f1a2 100644
--- a/sound/core/control_led.c
+++ b/sound/core/control_led.c
@@ -140,6 +140,7 @@ static void snd_ctl_led_set_state(struct snd_card *card, unsigned int access,
 {
 	struct snd_ctl_led *led;
 	struct snd_ctl_led_ctl *lctl;
+	enum snd_ctl_led_mode mode;
 	int route;
 	bool found;
 
@@ -168,8 +169,9 @@ static void snd_ctl_led_set_state(struct snd_card *card, unsigned int access,
 				UPDATE_ROUTE(route, snd_ctl_led_get(lctl));
 			}
 		}
+		mode = led->mode;
 	}
-	switch (led->mode) {
+	switch (mode) {
 	case MODE_OFF:		route = 1; break;
 	case MODE_ON:		route = 0; break;
 	case MODE_FOLLOW_ROUTE:	if (route >= 0) route ^= 1; break;
@@ -382,6 +384,8 @@ static ssize_t mode_show(struct device *dev,
 	struct snd_ctl_led *led = container_of(dev, struct snd_ctl_led, dev);
 	const char *str = NULL;
 
+	guard(mutex)(&snd_ctl_led_mutex);
+
 	switch (led->mode) {
 	case MODE_FOLLOW_MUTE:	str = "follow-mute"; break;
 	case MODE_FOLLOW_ROUTE:	str = "follow-route"; break;
-- 
2.55.0

Re: [PATCH v2] ALSA: control: Synchronize LED mode accesses

[Takashi Iwai]

On Fri, 06 Nov 2026 19:39:53 +0100,
Sreeraj S Kurup wrote:
> 
> mode_store() updates led->mode while holding
> snd_ctl_led_mutex.
> 
> However, snd_ctl_led_set_state() and mode_show() read this
> field without holding the same mutex.
> 
> This can cause a data race when userspace changes the mode
> while another path reads it.
> 
> Read the mode under snd_ctl_led_mutex in
> snd_ctl_led_set_state() and use the local value after
> releasing the mutex.
> 
> Also protect the mode read in mode_show() with the mutex.
> 
> Fixes: cb17fe0045aa ("ALSA: control - add sysfs support to the LED trigger module")
> Signed-off-by: Sreeraj S Kurup <sreekuttan2156239@gmail.com>

As this sort of data race for a single value has no much impact on the
actual behavior, it's more or less just for consistency.
So I applied to for-next branch.  Thanks.


Takashi

Re: [PATCH] ALSA: control: Synchronize LED mode accesses

[Sreeraj S Kurup]

Hi,

Thanks for applying the patch!

Best regards,
Sreeraj

Re: [PATCH v2] ALSA: control: Synchronize LED mode accesses

[Sreeraj S Kurup]

Hi,

Thanks for applying the patch!

Best regards,
Sreeraj

Re: [PATCH v2] ALSA: control: Synchronize LED mode accesses

[Sreeraj S Kurup]

Hi Takashi,

I would like to kindly follow up on my first patch.

Could you please let me know if you have had a chance to review it?

For reference, here is the patch link:
https://lore.kernel.org/all/20261106162357.3616-1-sreekuttan2156239@gmail.com/

I would appreciate your feedback whenever you have time.

Thank you for your time and consideration.

Best regards,
Sreeraj

Re: [PATCH v2] ALSA: control: Synchronize LED mode accesses

[Takashi Iwai]

On Sat, 07 Nov 2026 13:12:35 +0100,
Sreeraj S Kurup wrote:
> 
> Hi Takashi,
> 
> I would like to kindly follow up on my first patch.
> 
> Could you please let me know if you have had a chance to review it?
> 
> For reference, here is the patch link:
> https://lore.kernel.org/all/20261106162357.3616-1-sreekuttan2156239@gmail.com/

I'm still considering what's the best way.

Your patch replaces and adds some open-code, which isn't nice.
OTOH, it's a small change, so it's acceptable.

An alternative would be to use simple_strtoul() that can return the
end pointer.  Or, we may reorganize the whole code a bit, so that to
get the delimiter at first (instead of the head of the next word),
then clear and parse.  But not sure whether it'd be worth.


Takashi