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: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c
index 92b0d4523a07a3b9d5af03da563182f94d9bdbda..6fe93d5f6f7142db546365aafffd3073a1a2d67d 100644 (file)
--- a/sound/core/seq/seq_clientmgr.c
+++ b/sound/core/seq/seq_clientmgr.c
@@ -564,7 +564,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);
@@ -1639,7 +1639,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 a9a0b2f2708e4ec3c0f442c1e35e1981862e60d7..ea1aa079627618ffae4ca3924a15c941bf40e06f 100644 (file)
--- a/sound/core/seq/seq_queue.c
+++ b/sound/core/seq/seq_queue.c
@@ -261,6 +261,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;
@@ -277,17 +279,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 0e1feb597586fbcab9278b4ed53367a294071c7d..bd5e5a5d52a8d459cc86a30e3c770a433918d9ad 100644 (file)
--- a/sound/core/seq/seq_timer.c
+++ b/sound/core/seq/seq_timer.c
@@ -436,14 +436,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);
@@ -460,7 +461,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 9506b661fe5bfcf056c97799a39e1ba9dec8f029..5d47d559465e6b3f8211fc602b509368546a4d16 100644 (file)
--- a/sound/core/seq/seq_timer.h
+++ b/sound/core/seq/seq_timer.h
@@ -135,7 +135,8 @@ int snd_seq_timer_set_ppq(struct snd_seq_timer *tmr, 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;