Commit 8af68e44 authored by Takashi Iwai's avatar Takashi Iwai Committed by Luis Henriques

ALSA: seq: Fix yet another races among ALSA timer accesses

commit 2cdc7b63 upstream.

ALSA sequencer may open/close and control ALSA timer instance
dynamically either via sequencer events or direct ioctls.  These are
done mostly asynchronously, and it may call still some timer action
like snd_timer_start() while another is calling snd_timer_close().
Since the instance gets removed by snd_timer_close(), it may lead to
a use-after-free.

This patch tries to address such a race by protecting each
snd_timer_*() call via the existing spinlock and also by avoiding the
access to timer during close call.

BugLink: http://lkml.kernel.org/r/CACT4Y+Z6RzW5MBr-HUdV-8zwg71WQfKTdPpYGvOeS7v4cyurNQ@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 b9b54e19
...@@ -92,6 +92,9 @@ void snd_seq_timer_delete(struct snd_seq_timer **tmr) ...@@ -92,6 +92,9 @@ void snd_seq_timer_delete(struct snd_seq_timer **tmr)
void snd_seq_timer_defaults(struct snd_seq_timer * tmr) void snd_seq_timer_defaults(struct snd_seq_timer * tmr)
{ {
unsigned long flags;
spin_lock_irqsave(&tmr->lock, flags);
/* setup defaults */ /* setup defaults */
tmr->ppq = 96; /* 96 PPQ */ tmr->ppq = 96; /* 96 PPQ */
tmr->tempo = 500000; /* 120 BPM */ tmr->tempo = 500000; /* 120 BPM */
...@@ -107,21 +110,25 @@ void snd_seq_timer_defaults(struct snd_seq_timer * tmr) ...@@ -107,21 +110,25 @@ void snd_seq_timer_defaults(struct snd_seq_timer * tmr)
tmr->preferred_resolution = seq_default_timer_resolution; tmr->preferred_resolution = seq_default_timer_resolution;
tmr->skew = tmr->skew_base = SKEW_BASE; tmr->skew = tmr->skew_base = SKEW_BASE;
spin_unlock_irqrestore(&tmr->lock, flags);
} }
void snd_seq_timer_reset(struct snd_seq_timer * tmr) static void seq_timer_reset(struct snd_seq_timer *tmr)
{ {
unsigned long flags;
spin_lock_irqsave(&tmr->lock, flags);
/* reset time & songposition */ /* reset time & songposition */
tmr->cur_time.tv_sec = 0; tmr->cur_time.tv_sec = 0;
tmr->cur_time.tv_nsec = 0; tmr->cur_time.tv_nsec = 0;
tmr->tick.cur_tick = 0; tmr->tick.cur_tick = 0;
tmr->tick.fraction = 0; tmr->tick.fraction = 0;
}
void snd_seq_timer_reset(struct snd_seq_timer *tmr)
{
unsigned long flags;
spin_lock_irqsave(&tmr->lock, flags);
seq_timer_reset(tmr);
spin_unlock_irqrestore(&tmr->lock, flags); spin_unlock_irqrestore(&tmr->lock, flags);
} }
...@@ -140,8 +147,11 @@ static void snd_seq_timer_interrupt(struct snd_timer_instance *timeri, ...@@ -140,8 +147,11 @@ static void snd_seq_timer_interrupt(struct snd_timer_instance *timeri,
tmr = q->timer; tmr = q->timer;
if (tmr == NULL) if (tmr == NULL)
return; return;
if (!tmr->running) spin_lock_irqsave(&tmr->lock, flags);
if (!tmr->running) {
spin_unlock_irqrestore(&tmr->lock, flags);
return; return;
}
resolution *= ticks; resolution *= ticks;
if (tmr->skew != tmr->skew_base) { if (tmr->skew != tmr->skew_base) {
...@@ -150,8 +160,6 @@ static void snd_seq_timer_interrupt(struct snd_timer_instance *timeri, ...@@ -150,8 +160,6 @@ static void snd_seq_timer_interrupt(struct snd_timer_instance *timeri,
(((resolution & 0xffff) * tmr->skew) >> 16); (((resolution & 0xffff) * tmr->skew) >> 16);
} }
spin_lock_irqsave(&tmr->lock, flags);
/* update timer */ /* update timer */
snd_seq_inc_time_nsec(&tmr->cur_time, resolution); snd_seq_inc_time_nsec(&tmr->cur_time, resolution);
...@@ -298,26 +306,30 @@ int snd_seq_timer_open(struct snd_seq_queue *q) ...@@ -298,26 +306,30 @@ int snd_seq_timer_open(struct snd_seq_queue *q)
t->callback = snd_seq_timer_interrupt; t->callback = snd_seq_timer_interrupt;
t->callback_data = q; t->callback_data = q;
t->flags |= SNDRV_TIMER_IFLG_AUTO; t->flags |= SNDRV_TIMER_IFLG_AUTO;
spin_lock_irq(&tmr->lock);
tmr->timeri = t; tmr->timeri = t;
spin_unlock_irq(&tmr->lock);
return 0; return 0;
} }
int snd_seq_timer_close(struct snd_seq_queue *q) int snd_seq_timer_close(struct snd_seq_queue *q)
{ {
struct snd_seq_timer *tmr; struct snd_seq_timer *tmr;
struct snd_timer_instance *t;
tmr = q->timer; tmr = q->timer;
if (snd_BUG_ON(!tmr)) if (snd_BUG_ON(!tmr))
return -EINVAL; return -EINVAL;
if (tmr->timeri) { spin_lock_irq(&tmr->lock);
snd_timer_stop(tmr->timeri); t = tmr->timeri;
snd_timer_close(tmr->timeri); tmr->timeri = NULL;
tmr->timeri = NULL; spin_unlock_irq(&tmr->lock);
} if (t)
snd_timer_close(t);
return 0; return 0;
} }
int snd_seq_timer_stop(struct snd_seq_timer * tmr) static int seq_timer_stop(struct snd_seq_timer *tmr)
{ {
if (! tmr->timeri) if (! tmr->timeri)
return -EINVAL; return -EINVAL;
...@@ -328,6 +340,17 @@ int snd_seq_timer_stop(struct snd_seq_timer * tmr) ...@@ -328,6 +340,17 @@ int snd_seq_timer_stop(struct snd_seq_timer * tmr)
return 0; return 0;
} }
int snd_seq_timer_stop(struct snd_seq_timer *tmr)
{
unsigned long flags;
int err;
spin_lock_irqsave(&tmr->lock, flags);
err = seq_timer_stop(tmr);
spin_unlock_irqrestore(&tmr->lock, flags);
return err;
}
static int initialize_timer(struct snd_seq_timer *tmr) static int initialize_timer(struct snd_seq_timer *tmr)
{ {
struct snd_timer *t; struct snd_timer *t;
...@@ -360,13 +383,13 @@ static int initialize_timer(struct snd_seq_timer *tmr) ...@@ -360,13 +383,13 @@ static int initialize_timer(struct snd_seq_timer *tmr)
return 0; return 0;
} }
int snd_seq_timer_start(struct snd_seq_timer * tmr) static int seq_timer_start(struct snd_seq_timer *tmr)
{ {
if (! tmr->timeri) if (! tmr->timeri)
return -EINVAL; return -EINVAL;
if (tmr->running) if (tmr->running)
snd_seq_timer_stop(tmr); seq_timer_stop(tmr);
snd_seq_timer_reset(tmr); seq_timer_reset(tmr);
if (initialize_timer(tmr) < 0) if (initialize_timer(tmr) < 0)
return -EINVAL; return -EINVAL;
snd_timer_start(tmr->timeri, tmr->ticks); snd_timer_start(tmr->timeri, tmr->ticks);
...@@ -375,14 +398,25 @@ int snd_seq_timer_start(struct snd_seq_timer * tmr) ...@@ -375,14 +398,25 @@ int snd_seq_timer_start(struct snd_seq_timer * tmr)
return 0; return 0;
} }
int snd_seq_timer_continue(struct snd_seq_timer * tmr) int snd_seq_timer_start(struct snd_seq_timer *tmr)
{
unsigned long flags;
int err;
spin_lock_irqsave(&tmr->lock, flags);
err = seq_timer_start(tmr);
spin_unlock_irqrestore(&tmr->lock, flags);
return err;
}
static int seq_timer_continue(struct snd_seq_timer *tmr)
{ {
if (! tmr->timeri) if (! tmr->timeri)
return -EINVAL; return -EINVAL;
if (tmr->running) if (tmr->running)
return -EBUSY; return -EBUSY;
if (! tmr->initialized) { if (! tmr->initialized) {
snd_seq_timer_reset(tmr); seq_timer_reset(tmr);
if (initialize_timer(tmr) < 0) if (initialize_timer(tmr) < 0)
return -EINVAL; return -EINVAL;
} }
...@@ -392,11 +426,24 @@ int snd_seq_timer_continue(struct snd_seq_timer * tmr) ...@@ -392,11 +426,24 @@ int snd_seq_timer_continue(struct snd_seq_timer * tmr)
return 0; return 0;
} }
int snd_seq_timer_continue(struct snd_seq_timer *tmr)
{
unsigned long flags;
int err;
spin_lock_irqsave(&tmr->lock, flags);
err = seq_timer_continue(tmr);
spin_unlock_irqrestore(&tmr->lock, flags);
return err;
}
/* return current 'real' time. use timeofday() to get better granularity. */ /* return current 'real' time. use timeofday() to get better granularity. */
snd_seq_real_time_t snd_seq_timer_get_cur_time(struct snd_seq_timer *tmr) snd_seq_real_time_t snd_seq_timer_get_cur_time(struct snd_seq_timer *tmr)
{ {
snd_seq_real_time_t cur_time; snd_seq_real_time_t cur_time;
unsigned long flags;
spin_lock_irqsave(&tmr->lock, flags);
cur_time = tmr->cur_time; cur_time = tmr->cur_time;
if (tmr->running) { if (tmr->running) {
struct timeval tm; struct timeval tm;
...@@ -412,7 +459,7 @@ snd_seq_real_time_t snd_seq_timer_get_cur_time(struct snd_seq_timer *tmr) ...@@ -412,7 +459,7 @@ snd_seq_real_time_t snd_seq_timer_get_cur_time(struct snd_seq_timer *tmr)
} }
snd_seq_sanity_real_time(&cur_time); snd_seq_sanity_real_time(&cur_time);
} }
spin_unlock_irqrestore(&tmr->lock, flags);
return cur_time; return cur_time;
} }
......
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