| From 1e80829bb7900e4223482abc9f8a3c0a1f30ea73 Mon Sep 17 00:00:00 2001 |
| From: Takashi Iwai <tiwai@suse.de> |
| Date: Fri, 14 Feb 2020 12:13:14 +0100 |
| Subject: [PATCH] ALSA: seq: Avoid concurrent access to queue flags |
| |
| commit bb51e669fa49feb5904f452b2991b240ef31bc97 upstream. |
| |
| The queue flags are represented in bit fields and the concurrent |
| access may result in unexpected results. Although the current code |
| should be mostly OK as it's only reading a field while writing other |
| fields as KCSAN reported, it's safer to cover both with a proper |
| spinlock protection. |
| |
| This patch fixes the possible concurrent read by protecting with |
| q->owner_lock. Also the queue owner field is protected as well since |
| it's the field to be protected by the lock itself. |
| |
| Reported-by: syzbot+65c6c92d04304d0a8efc@syzkaller.appspotmail.com |
| Reported-by: syzbot+e60ddfa48717579799dd@syzkaller.appspotmail.com |
| Link: https://lore.kernel.org/r/20200214111316.26939-2-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_queue.c b/sound/core/seq/seq_queue.c |
| index caf68bf42f13..20c552cf8398 100644 |
| --- a/sound/core/seq/seq_queue.c |
| +++ b/sound/core/seq/seq_queue.c |
| @@ -392,6 +392,7 @@ int snd_seq_queue_check_access(int queueid, int client) |
| int snd_seq_queue_set_owner(int queueid, int client, int locked) |
| { |
| struct snd_seq_queue *q = queueptr(queueid); |
| + unsigned long flags; |
| |
| if (q == NULL) |
| return -EINVAL; |
| @@ -401,8 +402,10 @@ int snd_seq_queue_set_owner(int queueid, int client, int locked) |
| return -EPERM; |
| } |
| |
| + spin_lock_irqsave(&q->owner_lock, flags); |
| q->locked = locked ? 1 : 0; |
| q->owner = client; |
| + spin_unlock_irqrestore(&q->owner_lock, flags); |
| queue_access_unlock(q); |
| queuefree(q); |
| |
| @@ -539,15 +542,17 @@ void snd_seq_queue_client_termination(int client) |
| unsigned long flags; |
| int i; |
| struct snd_seq_queue *q; |
| + bool matched; |
| |
| for (i = 0; i < SNDRV_SEQ_MAX_QUEUES; i++) { |
| if ((q = queueptr(i)) == NULL) |
| continue; |
| spin_lock_irqsave(&q->owner_lock, flags); |
| - if (q->owner == client) |
| + matched = (q->owner == client); |
| + if (matched) |
| q->klocked = 1; |
| spin_unlock_irqrestore(&q->owner_lock, flags); |
| - if (q->owner == client) { |
| + if (matched) { |
| if (q->timer->running) |
| snd_seq_timer_stop(q->timer); |
| snd_seq_timer_reset(q->timer); |
| @@ -739,6 +744,8 @@ void snd_seq_info_queues_read(struct snd_info_entry *entry, |
| int i, bpm; |
| struct snd_seq_queue *q; |
| struct snd_seq_timer *tmr; |
| + bool locked; |
| + int owner; |
| |
| for (i = 0; i < SNDRV_SEQ_MAX_QUEUES; i++) { |
| if ((q = queueptr(i)) == NULL) |
| @@ -750,9 +757,14 @@ void snd_seq_info_queues_read(struct snd_info_entry *entry, |
| else |
| bpm = 0; |
| |
| + spin_lock_irq(&q->owner_lock); |
| + locked = q->locked; |
| + owner = q->owner; |
| + spin_unlock_irq(&q->owner_lock); |
| + |
| snd_iprintf(buffer, "queue %d: [%s]\n", q->queue, q->name); |
| - snd_iprintf(buffer, "owned by client : %d\n", q->owner); |
| - snd_iprintf(buffer, "lock status : %s\n", q->locked ? "Locked" : "Free"); |
| + snd_iprintf(buffer, "owned by client : %d\n", owner); |
| + snd_iprintf(buffer, "lock status : %s\n", locked ? "Locked" : "Free"); |
| snd_iprintf(buffer, "queued time events : %d\n", snd_seq_prioq_avail(q->timeq)); |
| snd_iprintf(buffer, "queued tick events : %d\n", snd_seq_prioq_avail(q->tickq)); |
| snd_iprintf(buffer, "timer state : %s\n", tmr->running ? "Running" : "Stopped"); |
| -- |
| 2.7.4 |
| |