ALSA: timer: Forcibly close timer instances at closing

When snd_timer object is freed via snd_timer_free() and still pending
snd_timer_instance objects are assigned to the timer object, it tries
to unlink all instances and just set NULL to each ti->timer, then
releases the resources immediately.  The problem is, however, when
there are slave timer instances that are associated with a master
instance linked to this timer: namely, those slave instances still
point to the freed timer object although the master instance is
unlinked, which may lead to user-after-free.  The bug can be easily
triggered particularly when a new userspace-driven timers
(CONFIG_SND_UTIMER) is involved, since it can create and delete the
timer object via a simple file open/close, while the other
applications may keep accessing to that timer.

This patch is an attempt to paper over the problem above: now instead
of just unlinking, call snd_timer_close[_locked]() forcibly for each
pending timer instance, so that all assigned slave timer instances are
properly detached, too.  Since snd_timer_close() might be called later
by the driver that created that instance, the check of
SNDRV_TIMER_IFLG_DEAD is added at the beginning, too.

Reported-by: Kyle Zeng <kylebot@openai.com>
Tested-by: Kyle Zeng <kylebot@openai.com>
Fixes: 37745918e0e7 ("ALSA: timer: Introduce virtual userspace-driven timers")
Cc: stable@vger.kernel.org
Link: https://patch.msgid.link/20260606161145.1933447-1-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>

diff --git a/sound/core/timer.c b/sound/core/timer.c
index 57583dec39748a12b487a78e041a6e2ca0f227e3..67fb1ecb33f0111955be10888bce239fde1295f1 100644 (file)
--- a/sound/core/timer.c
+++ b/sound/core/timer.c
@@ -430,6 +430,8 @@ static void snd_timer_close_locked(struct snd_timer_instance *timeri,
 
        if (timer) {
                guard(spinlock_irq)(&timer->lock);
+               if (timeri->flags & SNDRV_TIMER_IFLG_DEAD)
+                       return; /* already closed */
                timeri->flags |= SNDRV_TIMER_IFLG_DEAD;
        }
 
@@ -975,18 +977,18 @@ EXPORT_SYMBOL(snd_timer_new);
 
 static int snd_timer_free(struct snd_timer *timer)
 {
+       struct snd_timer_instance *ti, *n;
+
        if (!timer)
                return 0;
 
        guard(mutex)(&register_mutex);
        if (! list_empty(&timer->open_list_head)) {
-               struct list_head *p, *n;
-               struct snd_timer_instance *ti;
-               pr_warn("ALSA: timer %p is busy?\n", timer);
-               list_for_each_safe(p, n, &timer->open_list_head) {
-                       list_del_init(p);
-                       ti = list_entry(p, struct snd_timer_instance, open_list);
-                       ti->timer = NULL;
+               list_for_each_entry_safe(ti, n, &timer->open_list_head, open_list) {
+                       struct device *card_dev_to_put = NULL;
+
+                       snd_timer_close_locked(ti, &card_dev_to_put);
+                       put_device(card_dev_to_put);
                }
        }
        list_del(&timer->device_list);