[PATCH 1/2] ALSA: timer: don't re-enter an instance callback that is still running

[PATCH 1/2] ALSA: timer: don't re-enter an instance callback that is still running

[Norbert Szetei]

The userspace-driven timer (utimer) TRIGGER ioctl calls
snd_timer_interrupt() directly with no serialization, so two threads
triggering the same utimer can run snd_timer_interrupt() on one
snd_timer concurrently.

snd_timer_process_callbacks() drops timer->lock around each instance
callback and marks the in-flight callback with the single
SNDRV_TIMER_IFLG_CALLBACK bit; snd_timer_close_locked() waits on that
bit to drain an in-flight callback before freeing the instance. The bit
cannot represent two concurrent callbacks: when a second interrupt
re-queues an instance whose callback is still running, both run at once,
the first to finish clears the bit, and the close-path drain then frees
the instance (and its callback_data) while the other callback is still
live - a use-after-free reachable by any user able to open
/dev/snd/timer, both via a user timer instance and via a sequencer queue
timer bound to the utimer.

snd_timer_interrupt() sets IFLG_CALLBACK before dropping timer->lock, so
a concurrent interrupt already observes it under the lock. Skip
re-queuing an instance (and its slaves) to the ack/sack list while its
callback is in flight; the accumulated pticks are delivered on the next
tick, so no event is lost.

Fixes: 37745918e0e7 ("ALSA: timer: Introduce virtual userspace-driven timers")
Cc: <stable@vger.kernel.org>
Suggested-by: Takashi Iwai <tiwai@suse.de>
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Norbert Szetei <norbert@doyensec.com>
---
 sound/core/timer.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/sound/core/timer.c b/sound/core/timer.c
--- a/sound/core/timer.c
+++ b/sound/core/timer.c
@@ -927,12 +927,15 @@ void snd_timer_interrupt(struct snd_timer * timer, unsigned long ticks_left)
 			ack_list_head = &timer->ack_list_head;
 		else
 			ack_list_head = &timer->sack_list_head;
-		if (list_empty(&ti->ack_list))
+		/* don't requeue an instance whose callback is still running */
+		if (list_empty(&ti->ack_list) &&
+		    !(ti->flags & SNDRV_TIMER_IFLG_CALLBACK))
 			list_add_tail(&ti->ack_list, ack_list_head);
 		list_for_each_entry(ts, &ti->slave_active_head, active_list) {
 			ts->pticks = ti->pticks;
 			ts->resolution = resolution;
-			if (list_empty(&ts->ack_list))
+			if (list_empty(&ts->ack_list) &&
+			    !(ts->flags & SNDRV_TIMER_IFLG_CALLBACK))
 				list_add_tail(&ts->ack_list, ack_list_head);
 		}
 	}
--
2.55.0

[PATCH 2/2] ALSA: timer: drain a slave's callback before its master detaches it

[Norbert Szetei]

snd_timer_close_locked() drains the closing instance's own in-flight
callback (IFLG_CALLBACK) before freeing it, but not its slaves'. When a
master instance is closed, remove_slave_links() clears each slave's
->timer; the slave's own close then reads timer == NULL and takes the
branch that skips the drain entirely (snd_timer_stop_slave() also no-ops
on a NULL timer). So a slave whose callback is still running when the
master is closed is freed underneath the live callback, leading to
use-after-free.

Drain the slaves too before remove_slave_links() severs them.
snd_timer_stop() has already taken this instance off the active list, so
no new slave callback can be queued. Take the slaves off the ack list so
a pending one can't fire either, then wait for any that is already in
flight.

Fixes: 37745918e0e7 ("ALSA: timer: Introduce virtual userspace-driven timers")
Cc: <stable@vger.kernel.org>
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Norbert Szetei <norbert@doyensec.com>
---
 sound/core/timer.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/sound/core/timer.c b/sound/core/timer.c
--- a/sound/core/timer.c
+++ b/sound/core/timer.c
@@ -489,10 +489,20 @@ static void snd_timer_close_locked(struct snd_timer_instance *timeri,
 	snd_timer_stop(timeri);

 	if (timer) {
+		struct snd_timer_instance *slave;
+		bool busy;
+
 		timer->num_instances--;
-		/* wait, until the active callback is finished */
+		/* unqueue then drain the slaves' callbacks before remove_slave_links() severs them */
 		spin_lock_irq(&timer->lock);
-		while (timeri->flags & SNDRV_TIMER_IFLG_CALLBACK) {
+		list_for_each_entry(slave, &timeri->slave_list_head, open_list)
+			list_del_init(&slave->ack_list);
+		for (;;) {
+			busy = timeri->flags & SNDRV_TIMER_IFLG_CALLBACK;
+			list_for_each_entry(slave, &timeri->slave_list_head, open_list)
+				busy |= slave->flags & SNDRV_TIMER_IFLG_CALLBACK;
+			if (!busy)
+				break;
 			spin_unlock_irq(&timer->lock);
 			udelay(10);
 			spin_lock_irq(&timer->lock);
 		}
 		spin_unlock_irq(&timer->lock);

 		remove_slave_links(timeri, timer);
--
2.55.0

Re: [PATCH 1/2] ALSA: timer: don't re-enter an instance callback that is still running

[Takashi Iwai]

On Mon, 20 Jul 2026 09:09:55 +0200,
Norbert Szetei wrote:
> 
> The userspace-driven timer (utimer) TRIGGER ioctl calls
> snd_timer_interrupt() directly with no serialization, so two threads
> triggering the same utimer can run snd_timer_interrupt() on one
> snd_timer concurrently.
> 
> snd_timer_process_callbacks() drops timer->lock around each instance
> callback and marks the in-flight callback with the single
> SNDRV_TIMER_IFLG_CALLBACK bit; snd_timer_close_locked() waits on that
> bit to drain an in-flight callback before freeing the instance. The bit
> cannot represent two concurrent callbacks: when a second interrupt
> re-queues an instance whose callback is still running, both run at once,
> the first to finish clears the bit, and the close-path drain then frees
> the instance (and its callback_data) while the other callback is still
> live - a use-after-free reachable by any user able to open
> /dev/snd/timer, both via a user timer instance and via a sequencer queue
> timer bound to the utimer.
> 
> snd_timer_interrupt() sets IFLG_CALLBACK before dropping timer->lock, so
> a concurrent interrupt already observes it under the lock. Skip
> re-queuing an instance (and its slaves) to the ack/sack list while its
> callback is in flight; the accumulated pticks are delivered on the next
> tick, so no event is lost.
> 
> Fixes: 37745918e0e7 ("ALSA: timer: Introduce virtual userspace-driven timers")
> Cc: <stable@vger.kernel.org>
> Suggested-by: Takashi Iwai <tiwai@suse.de>
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Norbert Szetei <norbert@doyensec.com>

Applied both patches now.  Thanks.


Takashi