| From: Takashi Iwai <tiwai@suse.de> |
| Date: Sat, 30 Jan 2016 23:30:25 +0100 |
| Subject: ALSA: seq: Fix yet another races among ALSA timer accesses |
| |
| commit 2cdc7b636d55cbcf42e1e6c8accd85e62d3e9ae8 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.com |
| Reported-by: Dmitry Vyukov <dvyukov@google.com> |
| Signed-off-by: Takashi Iwai <tiwai@suse.de> |
| Signed-off-by: Ben Hutchings <ben@decadent.org.uk> |
| --- |
| sound/core/seq/seq_timer.c | 87 +++++++++++++++++++++++++++++++++++----------- |
| 1 file changed, 67 insertions(+), 20 deletions(-) |
| |
| --- a/sound/core/seq/seq_timer.c |
| +++ b/sound/core/seq/seq_timer.c |
| @@ -92,6 +92,9 @@ void snd_seq_timer_delete(struct snd_seq |
| |
| void snd_seq_timer_defaults(struct snd_seq_timer * tmr) |
| { |
| + unsigned long flags; |
| + |
| + spin_lock_irqsave(&tmr->lock, flags); |
| /* setup defaults */ |
| tmr->ppq = 96; /* 96 PPQ */ |
| tmr->tempo = 500000; /* 120 BPM */ |
| @@ -107,21 +110,25 @@ void snd_seq_timer_defaults(struct snd_s |
| tmr->preferred_resolution = seq_default_timer_resolution; |
| |
| 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 */ |
| tmr->cur_time.tv_sec = 0; |
| tmr->cur_time.tv_nsec = 0; |
| |
| tmr->tick.cur_tick = 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); |
| } |
| |
| @@ -140,8 +147,11 @@ static void snd_seq_timer_interrupt(stru |
| tmr = q->timer; |
| if (tmr == NULL) |
| return; |
| - if (!tmr->running) |
| + spin_lock_irqsave(&tmr->lock, flags); |
| + if (!tmr->running) { |
| + spin_unlock_irqrestore(&tmr->lock, flags); |
| return; |
| + } |
| |
| resolution *= ticks; |
| if (tmr->skew != tmr->skew_base) { |
| @@ -150,8 +160,6 @@ static void snd_seq_timer_interrupt(stru |
| (((resolution & 0xffff) * tmr->skew) >> 16); |
| } |
| |
| - spin_lock_irqsave(&tmr->lock, flags); |
| - |
| /* update timer */ |
| snd_seq_inc_time_nsec(&tmr->cur_time, resolution); |
| |
| @@ -298,26 +306,30 @@ int snd_seq_timer_open(struct snd_seq_qu |
| t->callback = snd_seq_timer_interrupt; |
| t->callback_data = q; |
| t->flags |= SNDRV_TIMER_IFLG_AUTO; |
| + spin_lock_irq(&tmr->lock); |
| tmr->timeri = t; |
| + spin_unlock_irq(&tmr->lock); |
| return 0; |
| } |
| |
| int snd_seq_timer_close(struct snd_seq_queue *q) |
| { |
| struct snd_seq_timer *tmr; |
| + struct snd_timer_instance *t; |
| |
| tmr = q->timer; |
| if (snd_BUG_ON(!tmr)) |
| return -EINVAL; |
| - if (tmr->timeri) { |
| - snd_timer_stop(tmr->timeri); |
| - snd_timer_close(tmr->timeri); |
| - tmr->timeri = NULL; |
| - } |
| + spin_lock_irq(&tmr->lock); |
| + t = tmr->timeri; |
| + tmr->timeri = NULL; |
| + spin_unlock_irq(&tmr->lock); |
| + if (t) |
| + snd_timer_close(t); |
| 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) |
| return -EINVAL; |
| @@ -328,6 +340,17 @@ int snd_seq_timer_stop(struct snd_seq_ti |
| 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) |
| { |
| struct snd_timer *t; |
| @@ -360,13 +383,13 @@ static int initialize_timer(struct snd_s |
| 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) |
| return -EINVAL; |
| if (tmr->running) |
| - snd_seq_timer_stop(tmr); |
| - snd_seq_timer_reset(tmr); |
| + seq_timer_stop(tmr); |
| + seq_timer_reset(tmr); |
| if (initialize_timer(tmr) < 0) |
| return -EINVAL; |
| snd_timer_start(tmr->timeri, tmr->ticks); |
| @@ -375,14 +398,25 @@ int snd_seq_timer_start(struct snd_seq_t |
| 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) |
| return -EINVAL; |
| if (tmr->running) |
| return -EBUSY; |
| if (! tmr->initialized) { |
| - snd_seq_timer_reset(tmr); |
| + seq_timer_reset(tmr); |
| if (initialize_timer(tmr) < 0) |
| return -EINVAL; |
| } |
| @@ -392,11 +426,24 @@ int snd_seq_timer_continue(struct snd_se |
| 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. */ |
| snd_seq_real_time_t snd_seq_timer_get_cur_time(struct snd_seq_timer *tmr) |
| { |
| snd_seq_real_time_t cur_time; |
| + unsigned long flags; |
| |
| + spin_lock_irqsave(&tmr->lock, flags); |
| cur_time = tmr->cur_time; |
| if (tmr->running) { |
| struct timeval tm; |
| @@ -412,7 +459,7 @@ snd_seq_real_time_t snd_seq_timer_get_cu |
| } |
| snd_seq_sanity_real_time(&cur_time); |
| } |
| - |
| + spin_unlock_irqrestore(&tmr->lock, flags); |
| return cur_time; |
| } |
| |