| From 321f9da006d5447113985ab4f07f419623012014 Mon Sep 17 00:00:00 2001 |
| From: Takashi Iwai <tiwai@suse.de> |
| Date: Fri, 14 Feb 2020 12:13:15 +0100 |
| Subject: [PATCH] ALSA: seq: Fix concurrent access to queue current tick/time |
| |
| commit dc7497795e014d84699c3b8809ed6df35352dd74 upstream. |
| |
| snd_seq_check_queue() passes the current tick and time of the given |
| queue as a pointer to snd_seq_prioq_cell_out(), but those might be |
| updated concurrently by the seq timer update. |
| |
| Fix it by retrieving the current tick and time via the proper helper |
| functions at first, and pass those values to snd_seq_prioq_cell_out() |
| later in the loops. |
| |
| snd_seq_timer_get_cur_time() takes a new argument and adjusts with the |
| current system time only when it's requested so; this update isn't |
| needed for snd_seq_check_queue(), as it's called either from the |
| interrupt handler or right after queuing. |
| |
| Also, snd_seq_timer_get_cur_tick() is changed to read the value in the |
| spinlock for the concurrency, too. |
| |
| Reported-by: syzbot+fd5e0eaa1a32999173b2@syzkaller.appspotmail.com |
| Link: https://lore.kernel.org/r/20200214111316.26939-3-tiwai@suse.de |
| Signed-off-by: Takashi Iwai <tiwai@suse.de> |
| Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com> |
| |
| diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c |
| index 6d9592f0ae1d..cc93157fa950 100644 |
| --- a/sound/core/seq/seq_clientmgr.c |
| +++ b/sound/core/seq/seq_clientmgr.c |
| @@ -580,7 +580,7 @@ static int update_timestamp_of_queue(struct snd_seq_event *event, |
| event->queue = queue; |
| event->flags &= ~SNDRV_SEQ_TIME_STAMP_MASK; |
| if (real_time) { |
| - event->time.time = snd_seq_timer_get_cur_time(q->timer); |
| + event->time.time = snd_seq_timer_get_cur_time(q->timer, true); |
| event->flags |= SNDRV_SEQ_TIME_STAMP_REAL; |
| } else { |
| event->time.tick = snd_seq_timer_get_cur_tick(q->timer); |
| @@ -1659,7 +1659,7 @@ static int snd_seq_ioctl_get_queue_status(struct snd_seq_client *client, |
| tmr = queue->timer; |
| status->events = queue->tickq->cells + queue->timeq->cells; |
| |
| - status->time = snd_seq_timer_get_cur_time(tmr); |
| + status->time = snd_seq_timer_get_cur_time(tmr, true); |
| status->tick = snd_seq_timer_get_cur_tick(tmr); |
| |
| status->running = tmr->running; |
| diff --git a/sound/core/seq/seq_queue.c b/sound/core/seq/seq_queue.c |
| index 20c552cf8398..71a6ea62c3be 100644 |
| --- a/sound/core/seq/seq_queue.c |
| +++ b/sound/core/seq/seq_queue.c |
| @@ -238,6 +238,8 @@ void snd_seq_check_queue(struct snd_seq_queue *q, int atomic, int hop) |
| { |
| unsigned long flags; |
| struct snd_seq_event_cell *cell; |
| + snd_seq_tick_time_t cur_tick; |
| + snd_seq_real_time_t cur_time; |
| |
| if (q == NULL) |
| return; |
| @@ -254,17 +256,18 @@ void snd_seq_check_queue(struct snd_seq_queue *q, int atomic, int hop) |
| |
| __again: |
| /* Process tick queue... */ |
| + cur_tick = snd_seq_timer_get_cur_tick(q->timer); |
| for (;;) { |
| - cell = snd_seq_prioq_cell_out(q->tickq, |
| - &q->timer->tick.cur_tick); |
| + cell = snd_seq_prioq_cell_out(q->tickq, &cur_tick); |
| if (!cell) |
| break; |
| snd_seq_dispatch_event(cell, atomic, hop); |
| } |
| |
| /* Process time queue... */ |
| + cur_time = snd_seq_timer_get_cur_time(q->timer, false); |
| for (;;) { |
| - cell = snd_seq_prioq_cell_out(q->timeq, &q->timer->cur_time); |
| + cell = snd_seq_prioq_cell_out(q->timeq, &cur_time); |
| if (!cell) |
| break; |
| snd_seq_dispatch_event(cell, atomic, hop); |
| diff --git a/sound/core/seq/seq_timer.c b/sound/core/seq/seq_timer.c |
| index 3bc6095df44d..0b43fc5fe349 100644 |
| --- a/sound/core/seq/seq_timer.c |
| +++ b/sound/core/seq/seq_timer.c |
| @@ -422,14 +422,15 @@ int snd_seq_timer_continue(struct snd_seq_timer *tmr) |
| } |
| |
| /* 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, |
| + bool adjust_ktime) |
| { |
| snd_seq_real_time_t cur_time; |
| unsigned long flags; |
| |
| spin_lock_irqsave(&tmr->lock, flags); |
| cur_time = tmr->cur_time; |
| - if (tmr->running) { |
| + if (adjust_ktime && tmr->running) { |
| struct timespec64 tm; |
| |
| ktime_get_ts64(&tm); |
| @@ -446,7 +447,13 @@ snd_seq_real_time_t snd_seq_timer_get_cur_time(struct snd_seq_timer *tmr) |
| high PPQ values) */ |
| snd_seq_tick_time_t snd_seq_timer_get_cur_tick(struct snd_seq_timer *tmr) |
| { |
| - return tmr->tick.cur_tick; |
| + snd_seq_tick_time_t cur_tick; |
| + unsigned long flags; |
| + |
| + spin_lock_irqsave(&tmr->lock, flags); |
| + cur_tick = tmr->tick.cur_tick; |
| + spin_unlock_irqrestore(&tmr->lock, flags); |
| + return cur_tick; |
| } |
| |
| |
| diff --git a/sound/core/seq/seq_timer.h b/sound/core/seq/seq_timer.h |
| index 66c3e344eae3..4bec57df8158 100644 |
| --- a/sound/core/seq/seq_timer.h |
| +++ b/sound/core/seq/seq_timer.h |
| @@ -120,7 +120,8 @@ int snd_seq_timer_set_tempo_ppq(struct snd_seq_timer *tmr, int tempo, int ppq); |
| int snd_seq_timer_set_position_tick(struct snd_seq_timer *tmr, snd_seq_tick_time_t position); |
| int snd_seq_timer_set_position_time(struct snd_seq_timer *tmr, snd_seq_real_time_t position); |
| int snd_seq_timer_set_skew(struct snd_seq_timer *tmr, unsigned int skew, unsigned int base); |
| -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, |
| + bool adjust_ktime); |
| snd_seq_tick_time_t snd_seq_timer_get_cur_tick(struct snd_seq_timer *tmr); |
| |
| extern int seq_default_timer_class; |
| -- |
| 2.7.4 |
| |