summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorTakashi Iwai <tiwai@suse.de>2021-06-10 17:20:59 +0200
committerTakashi Iwai <tiwai@suse.de>2021-06-10 17:21:30 +0200
commit83e197a8414c0ba545e7e3916ce05f836f349273 (patch)
tree905362ad82a2d2ad6e99b618956ee96dae625a83
parenta0309c344886c499b6071e7f03658e7f71a9afbb (diff)
ALSA: seq: Fix race of snd_seq_timer_open()
The timer instance per queue is exclusive, and snd_seq_timer_open() should have managed the concurrent accesses. It looks as if it's checking the already existing timer instance at the beginning, but it's not right, because there is no protection, hence any later concurrent call of snd_seq_timer_open() may override the timer instance easily. This may result in UAF, as the leftover timer instance can keep running while the queue itself gets closed, as spotted by syzkaller recently. For avoiding the race, add a proper check at the assignment of tmr->timeri again, and return -EBUSY if it's been already registered. Reported-by: syzbot+ddc1260a83ed1cbf6fb5@syzkaller.appspotmail.com Cc: <stable@vger.kernel.org> Link: https://lore.kernel.org/r/000000000000dce34f05c42f110c@google.com Link: https://lore.kernel.org/r/20210610152059.24633-1-tiwai@suse.de Signed-off-by: Takashi Iwai <tiwai@suse.de>
-rw-r--r--sound/core/seq/seq_timer.c10
1 files changed, 9 insertions, 1 deletions
diff --git a/sound/core/seq/seq_timer.c b/sound/core/seq/seq_timer.c
index 1645e4142e30..9863be6fd43e 100644
--- a/sound/core/seq/seq_timer.c
+++ b/sound/core/seq/seq_timer.c
@@ -297,8 +297,16 @@ int snd_seq_timer_open(struct snd_seq_queue *q)
return err;
}
spin_lock_irq(&tmr->lock);
- tmr->timeri = t;
+ if (tmr->timeri)
+ err = -EBUSY;
+ else
+ tmr->timeri = t;
spin_unlock_irq(&tmr->lock);
+ if (err < 0) {
+ snd_timer_close(t);
+ snd_timer_instance_free(t);
+ return err;
+ }
return 0;
}