[PATCH] ALSA: firewire-motu: Protect register DSP event queue positions

[PATCH] ALSA: firewire-motu: Protect register DSP event queue positions

[Cássio Gabriel]

The register DSP event queue is updated under parser->lock, but
snd_motu_register_dsp_message_parser_count_event() reads pull_pos and
push_pos without the lock.
snd_motu_register_dsp_message_parser_copy_event() also reads both queue
positions before taking the lock.

Protect these accesses with parser->lock as well. This keeps the hwdep
poll/read path consistent with the producer side and with the cached
meter/parameter accessors.

Fixes: 634ec0b2906e ("ALSA: firewire-motu: notify event for parameter change in register DSP model")
Cc: stable@vger.kernel.org
Signed-off-by: Cássio Gabriel <cassiogabrielcontato@gmail.com>
---
 sound/firewire/motu/motu-register-dsp-message-parser.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/sound/firewire/motu/motu-register-dsp-message-parser.c b/sound/firewire/motu/motu-register-dsp-message-parser.c
index a8053e3ef065..4ec23e6880d9 100644
--- a/sound/firewire/motu/motu-register-dsp-message-parser.c
+++ b/sound/firewire/motu/motu-register-dsp-message-parser.c
@@ -386,6 +386,8 @@ unsigned int snd_motu_register_dsp_message_parser_count_event(struct snd_motu *m
 {
 	struct msg_parser *parser = motu->message_parser;
 
+	guard(spinlock_irqsave)(&parser->lock);
+
 	if (parser->pull_pos > parser->push_pos)
 		return EVENT_QUEUE_SIZE - parser->pull_pos + parser->push_pos;
 	else
@@ -395,13 +397,14 @@ unsigned int snd_motu_register_dsp_message_parser_count_event(struct snd_motu *m
 bool snd_motu_register_dsp_message_parser_copy_event(struct snd_motu *motu, u32 *event)
 {
 	struct msg_parser *parser = motu->message_parser;
-	unsigned int pos = parser->pull_pos;
-
-	if (pos == parser->push_pos)
-		return false;
+	unsigned int pos;
 
 	guard(spinlock_irqsave)(&parser->lock);
 
+	if (parser->pull_pos == parser->push_pos)
+		return false;
+
+	pos = parser->pull_pos;
 	*event = parser->event_queue[pos];
 
 	++pos;

---
base-commit: 38c607c673155d6335591cdbd9c785fb2b7550e5
change-id: 20260501-alsa-firewire-motu-event-locking-d159b967a62c

Best regards,
--  
Cássio Gabriel <cassiogabrielcontato@gmail.com>

Re: [PATCH] ALSA: firewire-motu: Protect register DSP event queue positions

[Takashi Sakamoto]

Hi,

On Thu, May 21, 2026 at 08:01:23AM -0300, Cássio Gabriel wrote:
> The register DSP event queue is updated under parser->lock, but
> snd_motu_register_dsp_message_parser_count_event() reads pull_pos and
> push_pos without the lock.
> snd_motu_register_dsp_message_parser_copy_event() also reads both queue
> positions before taking the lock.
> 
> Protect these accesses with parser->lock as well. This keeps the hwdep
> poll/read path consistent with the producer side and with the cached
> meter/parameter accessors.
> 
> Fixes: 634ec0b2906e ("ALSA: firewire-motu: notify event for parameter change in register DSP model")
> Cc: stable@vger.kernel.org
> Signed-off-by: Cássio Gabriel <cassiogabrielcontato@gmail.com>
> ---
>  sound/firewire/motu/motu-register-dsp-message-parser.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)

Reviewed-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>

I remember that I was investigating whether to use the
'snd_motu.hwdep_lock' spin lock when working for this code,
especially for 'pull_pos'. The 'snd_hwdep.exclusive' is used to constraint
the number of file descriptors to open it, and the spin lock would be
protect the concurrent access, since the pull_pos is referred and updated
by user process only.  However,
snd_motu_register_dsp_message_parser_copy_event() is called outside of
the spin lock. The suggested change is preferable.


Thanks

Takashi Sakamoto

Re: [PATCH] ALSA: firewire-motu: Protect register DSP event queue positions

[Takashi Iwai]

On Thu, 21 May 2026 13:01:23 +0200,
Cássio Gabriel wrote:
> 
> The register DSP event queue is updated under parser->lock, but
> snd_motu_register_dsp_message_parser_count_event() reads pull_pos and
> push_pos without the lock.
> snd_motu_register_dsp_message_parser_copy_event() also reads both queue
> positions before taking the lock.
> 
> Protect these accesses with parser->lock as well. This keeps the hwdep
> poll/read path consistent with the producer side and with the cached
> meter/parameter accessors.
> 
> Fixes: 634ec0b2906e ("ALSA: firewire-motu: notify event for parameter change in register DSP model")
> Cc: stable@vger.kernel.org
> Signed-off-by: Cássio Gabriel <cassiogabrielcontato@gmail.com>

Applied now.  Thanks.


Takashi