Commit 95cd42a4 authored by Takashi Iwai's avatar Takashi Iwai Committed by Luis Henriques

ALSA: timer: Fix link corruption due to double start or stop

commit f784beb7 upstream.

Although ALSA timer code got hardening for races, it still causes
use-after-free error.  This is however rather a corrupted linked list,
not actually the concurrent accesses.  Namely, when timer start is
triggered twice, list_add_tail() is called twice, too.  This ends
up with the link corruption and triggers KASAN error.

The simplest fix would be replacing list_add_tail() with
list_move_tail(), but fundamentally it's the problem that we don't
check the double start/stop correctly.  So, the right fix here is to
add the proper checks to snd_timer_start() and snd_timer_stop() (and
their variants).

BugLink: http://lkml.kernel.org/r/CACT4Y+ZyPRoMQjmawbvmCEDrkBD2BQuH7R09=eOkf5ESK8kJAw@mail.gmail.comReported-by: default avatarDmitry Vyukov <dvyukov@google.com>
Signed-off-by: default avatarTakashi Iwai <tiwai@suse.de>
Signed-off-by: default avatarLuis Henriques <luis.henriques@canonical.com>
parent a293ce87
...@@ -451,6 +451,10 @@ static int snd_timer_start_slave(struct snd_timer_instance *timeri) ...@@ -451,6 +451,10 @@ static int snd_timer_start_slave(struct snd_timer_instance *timeri)
unsigned long flags; unsigned long flags;
spin_lock_irqsave(&slave_active_lock, flags); spin_lock_irqsave(&slave_active_lock, flags);
if (timeri->flags & SNDRV_TIMER_IFLG_RUNNING) {
spin_unlock_irqrestore(&slave_active_lock, flags);
return -EBUSY;
}
timeri->flags |= SNDRV_TIMER_IFLG_RUNNING; timeri->flags |= SNDRV_TIMER_IFLG_RUNNING;
if (timeri->master && timeri->timer) { if (timeri->master && timeri->timer) {
spin_lock(&timeri->timer->lock); spin_lock(&timeri->timer->lock);
...@@ -475,6 +479,7 @@ int snd_timer_start(struct snd_timer_instance *timeri, unsigned int ticks) ...@@ -475,6 +479,7 @@ int snd_timer_start(struct snd_timer_instance *timeri, unsigned int ticks)
return -EINVAL; return -EINVAL;
if (timeri->flags & SNDRV_TIMER_IFLG_SLAVE) { if (timeri->flags & SNDRV_TIMER_IFLG_SLAVE) {
result = snd_timer_start_slave(timeri); result = snd_timer_start_slave(timeri);
if (result >= 0)
snd_timer_notify1(timeri, SNDRV_TIMER_EVENT_START); snd_timer_notify1(timeri, SNDRV_TIMER_EVENT_START);
return result; return result;
} }
...@@ -484,10 +489,17 @@ int snd_timer_start(struct snd_timer_instance *timeri, unsigned int ticks) ...@@ -484,10 +489,17 @@ int snd_timer_start(struct snd_timer_instance *timeri, unsigned int ticks)
if (timer->card && timer->card->shutdown) if (timer->card && timer->card->shutdown)
return -ENODEV; return -ENODEV;
spin_lock_irqsave(&timer->lock, flags); spin_lock_irqsave(&timer->lock, flags);
if (timeri->flags & (SNDRV_TIMER_IFLG_RUNNING |
SNDRV_TIMER_IFLG_START)) {
result = -EBUSY;
goto unlock;
}
timeri->ticks = timeri->cticks = ticks; timeri->ticks = timeri->cticks = ticks;
timeri->pticks = 0; timeri->pticks = 0;
result = snd_timer_start1(timer, timeri, ticks); result = snd_timer_start1(timer, timeri, ticks);
unlock:
spin_unlock_irqrestore(&timer->lock, flags); spin_unlock_irqrestore(&timer->lock, flags);
if (result >= 0)
snd_timer_notify1(timeri, SNDRV_TIMER_EVENT_START); snd_timer_notify1(timeri, SNDRV_TIMER_EVENT_START);
return result; return result;
} }
...@@ -502,6 +514,10 @@ static int _snd_timer_stop(struct snd_timer_instance *timeri, int event) ...@@ -502,6 +514,10 @@ static int _snd_timer_stop(struct snd_timer_instance *timeri, int event)
if (timeri->flags & SNDRV_TIMER_IFLG_SLAVE) { if (timeri->flags & SNDRV_TIMER_IFLG_SLAVE) {
spin_lock_irqsave(&slave_active_lock, flags); spin_lock_irqsave(&slave_active_lock, flags);
if (!(timeri->flags & SNDRV_TIMER_IFLG_RUNNING)) {
spin_unlock_irqrestore(&slave_active_lock, flags);
return -EBUSY;
}
timeri->flags &= ~SNDRV_TIMER_IFLG_RUNNING; timeri->flags &= ~SNDRV_TIMER_IFLG_RUNNING;
list_del_init(&timeri->ack_list); list_del_init(&timeri->ack_list);
list_del_init(&timeri->active_list); list_del_init(&timeri->active_list);
...@@ -512,6 +528,11 @@ static int _snd_timer_stop(struct snd_timer_instance *timeri, int event) ...@@ -512,6 +528,11 @@ static int _snd_timer_stop(struct snd_timer_instance *timeri, int event)
if (!timer) if (!timer)
return -EINVAL; return -EINVAL;
spin_lock_irqsave(&timer->lock, flags); spin_lock_irqsave(&timer->lock, flags);
if (!(timeri->flags & (SNDRV_TIMER_IFLG_RUNNING |
SNDRV_TIMER_IFLG_START))) {
spin_unlock_irqrestore(&timer->lock, flags);
return -EBUSY;
}
list_del_init(&timeri->ack_list); list_del_init(&timeri->ack_list);
list_del_init(&timeri->active_list); list_del_init(&timeri->active_list);
if (timer->card && timer->card->shutdown) { if (timer->card && timer->card->shutdown) {
...@@ -581,10 +602,15 @@ int snd_timer_continue(struct snd_timer_instance *timeri) ...@@ -581,10 +602,15 @@ int snd_timer_continue(struct snd_timer_instance *timeri)
if (timer->card && timer->card->shutdown) if (timer->card && timer->card->shutdown)
return -ENODEV; return -ENODEV;
spin_lock_irqsave(&timer->lock, flags); spin_lock_irqsave(&timer->lock, flags);
if (timeri->flags & SNDRV_TIMER_IFLG_RUNNING) {
result = -EBUSY;
goto unlock;
}
if (!timeri->cticks) if (!timeri->cticks)
timeri->cticks = 1; timeri->cticks = 1;
timeri->pticks = 0; timeri->pticks = 0;
result = snd_timer_start1(timer, timeri, timer->sticks); result = snd_timer_start1(timer, timeri, timer->sticks);
unlock:
spin_unlock_irqrestore(&timer->lock, flags); spin_unlock_irqrestore(&timer->lock, flags);
snd_timer_notify1(timeri, SNDRV_TIMER_EVENT_CONTINUE); snd_timer_notify1(timeri, SNDRV_TIMER_EVENT_CONTINUE);
return result; return result;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment