ALSA: seq: Fix race at creating a queue

commit 4842e98f26dd80be3623c4714a244ba52ea096a8 upstream.

When a sequencer queue is created in snd_seq_queue_alloc(),it adds the
new queue element to the public list before referencing it.  Thus the
queue might be deleted before the call of snd_seq_queue_use(), and it
results in the use-after-free error, as spotted by syzkaller.

The fix is to reference the queue object at the right time.

Reported-by: Dmitry Vyukov <dvyukov@google.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>

diff --git a/sound/core/seq/seq_queue.c b/sound/core/seq/seq_queue.c
index 4c9aa46..17fe04d 100644 (file)
--- a/sound/core/seq/seq_queue.c
+++ b/sound/core/seq/seq_queue.c
@@ -183,6 +183,8 @@ void __exit snd_seq_queues_delete(void)
        }
 }
 
+static void queue_use(struct snd_seq_queue *queue, int client, int use);
+
 /* allocate a new queue -
  * return queue index value or negative value for error
  */
@@ -194,11 +196,11 @@ int snd_seq_queue_alloc(int client, int locked, unsigned int info_flags)
        if (q == NULL)
                return -ENOMEM;
        q->info_flags = info_flags;
+       queue_use(q, client, 1);
        if (queue_list_add(q) < 0) {
                queue_delete(q);
                return -ENOMEM;
        }
-       snd_seq_queue_use(q->queue, client, 1); /* use this queue */
        return q->queue;
 }
 
@@ -504,19 +506,9 @@ int snd_seq_queue_timer_set_tempo(int queueid, int client,
        return result;
 }
 
-
-/* use or unuse this queue -
- * if it is the first client, starts the timer.
- * if it is not longer used by any clients, stop the timer.
- */
-int snd_seq_queue_use(int queueid, int client, int use)
+/* use or unuse this queue */
+static void queue_use(struct snd_seq_queue *queue, int client, int use)
 {
-       struct snd_seq_queue *queue;
-
-       queue = queueptr(queueid);
-       if (queue == NULL)
-               return -EINVAL;
-       mutex_lock(&queue->timer_mutex);
        if (use) {
                if (!test_and_set_bit(client, queue->clients_bitmap))
                        queue->clients++;
@@ -531,6 +523,21 @@ int snd_seq_queue_use(int queueid, int client, int use)
        } else {
                snd_seq_timer_close(queue);
        }
+}
+
+/* use or unuse this queue -
+ * if it is the first client, starts the timer.
+ * if it is not longer used by any clients, stop the timer.
+ */
+int snd_seq_queue_use(int queueid, int client, int use)
+{
+       struct snd_seq_queue *queue;
+
+       queue = queueptr(queueid);
+       if (queue == NULL)
+               return -EINVAL;
+       mutex_lock(&queue->timer_mutex);
+       queue_use(queue, client, use);
        mutex_unlock(&queue->timer_mutex);
        queuefree(queue);
        return 0;