ALSA: timer: Assure timer resolution access always locked

There are still many places calling the timer's hw.c_resolution
callback without lock, and this may lead to some races, as we faced in
the commit a820ccbe21e8 ("ALSA: pcm: Fix UAF at PCM release via PCM
timer access").

This patch changes snd_timer_resolution() to take the timer->lock for
avoiding the races.  A place calling this function already inside the
lock (from the notifier) is replaced with the
snd_timer_hw_resolution() accordingly, as well as wrapping with the
lock around another place calling snd_timer_hw_resolution(), too.

Reported-by: Ben Hutchings <ben.hutchings@codethink.co.uk>
Signed-off-by: Takashi Iwai <tiwai@suse.de>

diff --git a/sound/core/timer.c b/sound/core/timer.c
index 22c72857f379974a3a28c1bc507e2f6a95e83a8c..665089c455603c0c683144c419981f8215780e85 100644 (file)
--- a/sound/core/timer.c
+++ b/sound/core/timer.c
@@ -438,19 +438,24 @@ static unsigned long snd_timer_hw_resolution(struct snd_timer *timer)
 unsigned long snd_timer_resolution(struct snd_timer_instance *timeri)
 {
        struct snd_timer * timer;
+       unsigned long ret = 0;
+       unsigned long flags;
 
        if (timeri == NULL)
                return 0;
        timer = timeri->timer;
-       if (timer)
-               return snd_timer_hw_resolution(timer);
-       return 0;
+       if (timer) {
+               spin_lock_irqsave(&timer->lock, flags);
+               ret = snd_timer_hw_resolution(timer);
+               spin_unlock_irqrestore(&timer->lock, flags);
+       }
+       return ret;
 }
 EXPORT_SYMBOL(snd_timer_resolution);
 
 static void snd_timer_notify1(struct snd_timer_instance *ti, int event)
 {
-       struct snd_timer *timer;
+       struct snd_timer *timer = ti->timer;
        unsigned long resolution = 0;
        struct snd_timer_instance *ts;
        struct timespec tstamp;
@@ -462,14 +467,14 @@ static void snd_timer_notify1(struct snd_timer_instance *ti, int event)
        if (snd_BUG_ON(event < SNDRV_TIMER_EVENT_START ||
                       event > SNDRV_TIMER_EVENT_PAUSE))
                return;
-       if (event == SNDRV_TIMER_EVENT_START ||
-           event == SNDRV_TIMER_EVENT_CONTINUE)
-               resolution = snd_timer_resolution(ti);
+       if (timer &&
+           (event == SNDRV_TIMER_EVENT_START ||
+            event == SNDRV_TIMER_EVENT_CONTINUE))
+               resolution = snd_timer_hw_resolution(timer);
        if (ti->ccallback)
                ti->ccallback(ti, event, &tstamp, resolution);
        if (ti->flags & SNDRV_TIMER_IFLG_SLAVE)
                return;
-       timer = ti->timer;
        if (timer == NULL)
                return;
        if (timer->hw.flags & SNDRV_TIMER_HW_SLAVE)
@@ -1654,6 +1659,7 @@ static int snd_timer_user_gstatus(struct file *file,
        mutex_lock(&register_mutex);
        t = snd_timer_find(&tid);
        if (t != NULL) {
+               spin_lock_irq(&t->lock);
                gstatus.resolution = snd_timer_hw_resolution(t);
                if (t->hw.precise_resolution) {
                        t->hw.precise_resolution(t, &gstatus.resolution_num,
@@ -1662,6 +1668,7 @@ static int snd_timer_user_gstatus(struct file *file,
                        gstatus.resolution_num = gstatus.resolution;
                        gstatus.resolution_den = 1000000000uL;
                }
+               spin_unlock_irq(&t->lock);
        } else {
                err = -ENODEV;
        }