[PATCH] ALSA: firewire-tascam: Do not drop unread control events

[PATCH] ALSA: firewire-tascam: Do not drop unread control events

[Cássio Gabriel]

tscm_hwdep_read_queue() copies as many queued control events as fit in
the userspace buffer. When the buffer is smaller than the current
contiguous queue segment, length is rounded down to the number of bytes
that can be copied.

However, after copying that shortened length, the code advances pull_pos
to tail_pos, marking the whole contiguous segment as consumed. Any events
between the copied portion and tail_pos are lost.

Advance pull_pos by the number of entries actually copied instead. When
the whole segment fits, this is equivalent to the old tail_pos update;
when the buffer is smaller, the remaining events stay queued for the next
read.

Fixes: a8c0d13267a4 ("ALSA: firewire-tascam: notify events of change of state for userspace applications")
Cc: stable@vger.kernel.org
Signed-off-by: Cássio Gabriel <cassiogabrielcontato@gmail.com>
---
 sound/firewire/tascam/tascam-hwdep.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/sound/firewire/tascam/tascam-hwdep.c b/sound/firewire/tascam/tascam-hwdep.c
index 867b4ea1096e..dcd3fbcbeb79 100644
--- a/sound/firewire/tascam/tascam-hwdep.c
+++ b/sound/firewire/tascam/tascam-hwdep.c
@@ -59,6 +59,7 @@ static long tscm_hwdep_read_queue(struct snd_tscm *tscm, char __user *buf,
 		unsigned int head_pos;
 		unsigned int tail_pos;
 		unsigned int length;
+		unsigned int entries_copied;
 
 		if (tscm->pull_pos == tscm->push_pos)
 			break;
@@ -73,6 +74,7 @@ static long tscm_hwdep_read_queue(struct snd_tscm *tscm, char __user *buf,
 			length = rounddown(remained, sizeof(*entries));
 		if (length == 0)
 			break;
+		entries_copied = length / sizeof(*entries);
 
 		spin_unlock_irq(&tscm->lock);
 		if (copy_to_user(pos, &entries[head_pos], length))
@@ -80,7 +82,7 @@ static long tscm_hwdep_read_queue(struct snd_tscm *tscm, char __user *buf,
 
 		spin_lock_irq(&tscm->lock);
 
-		tscm->pull_pos = tail_pos % SND_TSCM_QUEUE_COUNT;
+		tscm->pull_pos = (head_pos + entries_copied) % SND_TSCM_QUEUE_COUNT;
 
 		count += length;
 		remained -= length;

---
base-commit: 9e8d6ddd7ecf2ad42d614243f86e50fcf0183b9e
change-id: 20260501-alsa-firewire-tascam-read-queue-7eaef1060adb

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

Re: [PATCH] ALSA: firewire-tascam: Do not drop unread control events

[Takashi Sakamoto]

Hi,

On Fri, May 01, 2026 at 03:22:58PM -0300, Cássio Gabriel wrote:
> tscm_hwdep_read_queue() copies as many queued control events as fit in
> the userspace buffer. When the buffer is smaller than the current
> contiguous queue segment, length is rounded down to the number of bytes
> that can be copied.
> 
> However, after copying that shortened length, the code advances pull_pos
> to tail_pos, marking the whole contiguous segment as consumed. Any events
> between the copied portion and tail_pos are lost.
> 
> Advance pull_pos by the number of entries actually copied instead. When
> the whole segment fits, this is equivalent to the old tail_pos update;
> when the buffer is smaller, the remaining events stay queued for the next
> read.
> 
> Fixes: a8c0d13267a4 ("ALSA: firewire-tascam: notify events of change of state for userspace applications")
> Cc: stable@vger.kernel.org
> Signed-off-by: Cássio Gabriel <cassiogabrielcontato@gmail.com>
> ---
>  sound/firewire/tascam/tascam-hwdep.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

Thank you for catching the issue.

I agree that this is indeed a bug. While your approach makes sense, I
wonder if it could be simplified by updating tail_pos instead.

Would you mind trying the following change?

======== 8< --------
diff --git a/sound/firewire/tascam/tascam-hwdep.c b/sound/firewire/tascam/tascam-hwdep.c
index 867b4ea1096e..6270263e7bf4 100644
--- a/sound/firewire/tascam/tascam-hwdep.c
+++ b/sound/firewire/tascam/tascam-hwdep.c
@@ -73,6 +73,7 @@ static long tscm_hwdep_read_queue(struct snd_tscm *tscm, char __user *buf,
                        length = rounddown(remained, sizeof(*entries));
                if (length == 0)
                        break;
+               tail_pos = head_pos + length / sizeof(*entries);

                spin_unlock_irq(&tscm->lock);
                if (copy_to_user(pos, &entries[head_pos], length))
======== 8< --------


Thanks

Takashi Sakamoto

Re: [PATCH] ALSA: firewire-tascam: Do not drop unread control events

[Cássio Gabriel Monteiro Pires]

[-- Attachment #1.1: Type: text/plain, Size: 2227 bytes --]

On 5/3/26 08:29, Takashi Sakamoto wrote:
> Hi,
> 
> On Fri, May 01, 2026 at 03:22:58PM -0300, Cássio Gabriel wrote:
>> tscm_hwdep_read_queue() copies as many queued control events as fit in
>> the userspace buffer. When the buffer is smaller than the current
>> contiguous queue segment, length is rounded down to the number of bytes
>> that can be copied.
>>
>> However, after copying that shortened length, the code advances pull_pos
>> to tail_pos, marking the whole contiguous segment as consumed. Any events
>> between the copied portion and tail_pos are lost.
>>
>> Advance pull_pos by the number of entries actually copied instead. When
>> the whole segment fits, this is equivalent to the old tail_pos update;
>> when the buffer is smaller, the remaining events stay queued for the next
>> read.
>>
>> Fixes: a8c0d13267a4 ("ALSA: firewire-tascam: notify events of change of state for userspace applications")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Cássio Gabriel <cassiogabrielcontato@gmail.com>
>> ---
>>  sound/firewire/tascam/tascam-hwdep.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> Thank you for catching the issue.
> 
> I agree that this is indeed a bug. While your approach makes sense, I
> wonder if it could be simplified by updating tail_pos instead.
> 
> Would you mind trying the following change?
> 
> ======== 8< --------
> diff --git a/sound/firewire/tascam/tascam-hwdep.c b/sound/firewire/tascam/tascam-hwdep.c
> index 867b4ea1096e..6270263e7bf4 100644
> --- a/sound/firewire/tascam/tascam-hwdep.c
> +++ b/sound/firewire/tascam/tascam-hwdep.c
> @@ -73,6 +73,7 @@ static long tscm_hwdep_read_queue(struct snd_tscm *tscm, char __user *buf,
>                         length = rounddown(remained, sizeof(*entries));
>                 if (length == 0)
>                         break;
> +               tail_pos = head_pos + length / sizeof(*entries);
> 
>                 spin_unlock_irq(&tscm->lock);
>                 if (copy_to_user(pos, &entries[head_pos], length))
> ======== 8< --------
> 
> 
> Thanks
> 
> Takashi Sakamoto

Hi,

Thank you for the suggestion.

I'll send a v2 patch.

-- 
Thanks,
Cássio


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]