[linux] 02/02: Add ALSA security fixes
debian-kernel at lists.debian.org
debian-kernel at lists.debian.org
Sun Feb 28 23:20:42 UTC 2016
This is an automated email from the git hooks/post-receive script.
benh pushed a commit to branch wheezy-security
in repository linux.
commit 308b4d743f27c41d531887bc3269c2390d85d35d
Author: Ben Hutchings <ben at decadent.org.uk>
Date: Sun Feb 28 23:14:28 2016 +0000
Add ALSA security fixes
---
debian/changelog | 6 ++
.../alsa-hrtimer-fix-stall-by-hrtimer_cancel.patch | 49 +++++++++
...missing-null-check-at-remove_events-ioctl.patch | 29 +++++
...lsa-seq-fix-race-at-timer-setup-and-close.patch | 33 ++++++
...sa-timer-fix-double-unlink-of-active_list.patch | 32 ++++++
.../alsa-timer-fix-race-among-timer-ioctls.patch | 117 +++++++++++++++++++++
...sa-timer-harden-slave-timer-list-handling.patch | 96 +++++++++++++++++
debian/patches/series | 6 ++
8 files changed, 368 insertions(+)
diff --git a/debian/changelog b/debian/changelog
index 5ee64f9..e6ba05c 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -23,6 +23,12 @@ linux (3.2.73-2+deb7u3) UNRELEASED; urgency=medium
* unix: correctly track in-flight fds in sending process user_struct
(CVE-2016-2550)
* USB: fix invalid memory access in hub_activate() (CVE-2015-8816)
+ * ALSA: seq: Fix missing NULL check at remove_events ioctl (CVE-2016-2543)
+ * ALSA: seq: Fix race at timer setup and close (CVE-2016-2544)
+ * ALSA: timer: Fix double unlink of active_list (CVE-2016-2545)
+ * ALSA: timer: Fix race among timer ioctls (CVE-2016-2546)
+ * ALSA: timer: Harden slave timer list handling (CVE-2016-2547, CVE-2016-2548)
+ * ALSA: hrtimer: Fix stall by hrtimer_cancel() (CVE-2016-2549)
[ Salvatore Bonaccorso ]
* unix: properly account for FDs passed over unix sockets (CVE-2013-4312)
diff --git a/debian/patches/bugfix/all/alsa-hrtimer-fix-stall-by-hrtimer_cancel.patch b/debian/patches/bugfix/all/alsa-hrtimer-fix-stall-by-hrtimer_cancel.patch
new file mode 100644
index 0000000..df4b6e7
--- /dev/null
+++ b/debian/patches/bugfix/all/alsa-hrtimer-fix-stall-by-hrtimer_cancel.patch
@@ -0,0 +1,49 @@
+From: Takashi Iwai <tiwai at suse.de>
+Date: Mon, 18 Jan 2016 13:52:47 +0100
+Subject: ALSA: hrtimer: Fix stall by hrtimer_cancel()
+Origin: https://git.kernel.org/linus/2ba1fe7a06d3624f9a7586d672b55f08f7c670f3
+
+hrtimer_cancel() waits for the completion from the callback, thus it
+must not be called inside the callback itself. This was already a
+problem in the past with ALSA hrtimer driver, and the early commit
+[fcfdebe70759: ALSA: hrtimer - Fix lock-up] tried to address it.
+
+However, the previous fix is still insufficient: it may still cause a
+lockup when the ALSA timer instance reprograms itself in its callback.
+Then it invokes the start function even in snd_timer_interrupt() that
+is called in hrtimer callback itself, results in a CPU stall. This is
+no hypothetical problem but actually triggered by syzkaller fuzzer.
+
+This patch tries to fix the issue again. Now we call
+hrtimer_try_to_cancel() at both start and stop functions so that it
+won't fall into a deadlock, yet giving some chance to cancel the queue
+if the functions have been called outside the callback. The proper
+hrtimer_cancel() is called in anyway at closing, so this should be
+enough.
+
+Reported-and-tested-by: Dmitry Vyukov <dvyukov at google.com>
+Signed-off-by: Takashi Iwai <tiwai at suse.de>
+Signed-off-by: Ben Hutchings <ben at decadent.org.uk>
+---
+ sound/core/hrtimer.c | 3 ++-
+ 1 file changed, 2 insertions(+), 1 deletion(-)
+
+--- a/sound/core/hrtimer.c
++++ b/sound/core/hrtimer.c
+@@ -90,7 +90,7 @@ static int snd_hrtimer_start(struct snd_
+ struct snd_hrtimer *stime = t->private_data;
+
+ atomic_set(&stime->running, 0);
+- hrtimer_cancel(&stime->hrt);
++ hrtimer_try_to_cancel(&stime->hrt);
+ hrtimer_start(&stime->hrt, ns_to_ktime(t->sticks * resolution),
+ HRTIMER_MODE_REL);
+ atomic_set(&stime->running, 1);
+@@ -101,6 +101,7 @@ static int snd_hrtimer_stop(struct snd_t
+ {
+ struct snd_hrtimer *stime = t->private_data;
+ atomic_set(&stime->running, 0);
++ hrtimer_try_to_cancel(&stime->hrt);
+ return 0;
+ }
+
diff --git a/debian/patches/bugfix/all/alsa-seq-fix-missing-null-check-at-remove_events-ioctl.patch b/debian/patches/bugfix/all/alsa-seq-fix-missing-null-check-at-remove_events-ioctl.patch
new file mode 100644
index 0000000..55a565d
--- /dev/null
+++ b/debian/patches/bugfix/all/alsa-seq-fix-missing-null-check-at-remove_events-ioctl.patch
@@ -0,0 +1,29 @@
+From: Takashi Iwai <tiwai at suse.de>
+Date: Tue, 12 Jan 2016 12:38:02 +0100
+Subject: ALSA: seq: Fix missing NULL check at remove_events ioctl
+Origin: https://git.kernel.org/linus/030e2c78d3a91dd0d27fef37e91950dde333eba1
+
+snd_seq_ioctl_remove_events() calls snd_seq_fifo_clear()
+unconditionally even if there is no FIFO assigned, and this leads to
+an Oops due to NULL dereference. The fix is just to add a proper NULL
+check.
+
+Reported-by: Dmitry Vyukov <dvyukov at google.com>
+Tested-by: Dmitry Vyukov <dvyukov at google.com>
+Signed-off-by: Takashi Iwai <tiwai at suse.de>
+Signed-off-by: Ben Hutchings <ben at decadent.org.uk>
+---
+ sound/core/seq/seq_clientmgr.c | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+
+--- a/sound/core/seq/seq_clientmgr.c
++++ b/sound/core/seq/seq_clientmgr.c
+@@ -1950,7 +1950,7 @@ static int snd_seq_ioctl_remove_events(s
+ * No restrictions so for a user client we can clear
+ * the whole fifo
+ */
+- if (client->type == USER_CLIENT)
++ if (client->type == USER_CLIENT && client->data.user.fifo)
+ snd_seq_fifo_clear(client->data.user.fifo);
+ }
+
diff --git a/debian/patches/bugfix/all/alsa-seq-fix-race-at-timer-setup-and-close.patch b/debian/patches/bugfix/all/alsa-seq-fix-race-at-timer-setup-and-close.patch
new file mode 100644
index 0000000..47c02ee
--- /dev/null
+++ b/debian/patches/bugfix/all/alsa-seq-fix-race-at-timer-setup-and-close.patch
@@ -0,0 +1,33 @@
+From: Takashi Iwai <tiwai at suse.de>
+Date: Tue, 12 Jan 2016 15:36:27 +0100
+Subject: ALSA: seq: Fix race at timer setup and close
+Origin: https://git.kernel.org/linus/3567eb6af614dac436c4b16a8d426f9faed639b3
+
+ALSA sequencer code has an open race between the timer setup ioctl and
+the close of the client. This was triggered by syzkaller fuzzer, and
+a use-after-free was caught there as a result.
+
+This patch papers over it by adding a proper queue->timer_mutex lock
+around the timer-related calls in the relevant code path.
+
+Reported-by: Dmitry Vyukov <dvyukov at google.com>
+Tested-by: Dmitry Vyukov <dvyukov at google.com>
+Signed-off-by: Takashi Iwai <tiwai at suse.de>
+Signed-off-by: Ben Hutchings <ben at decadent.org.uk>
+---
+ sound/core/seq/seq_queue.c | 2 ++
+ 1 file changed, 2 insertions(+)
+
+--- a/sound/core/seq/seq_queue.c
++++ b/sound/core/seq/seq_queue.c
+@@ -144,8 +144,10 @@ static struct snd_seq_queue *queue_new(i
+ static void queue_delete(struct snd_seq_queue *q)
+ {
+ /* stop and release the timer */
++ mutex_lock(&q->timer_mutex);
+ snd_seq_timer_stop(q->timer);
+ snd_seq_timer_close(q);
++ mutex_unlock(&q->timer_mutex);
+ /* wait until access free */
+ snd_use_lock_sync(&q->use_lock);
+ /* release resources... */
diff --git a/debian/patches/bugfix/all/alsa-timer-fix-double-unlink-of-active_list.patch b/debian/patches/bugfix/all/alsa-timer-fix-double-unlink-of-active_list.patch
new file mode 100644
index 0000000..2bfef13
--- /dev/null
+++ b/debian/patches/bugfix/all/alsa-timer-fix-double-unlink-of-active_list.patch
@@ -0,0 +1,32 @@
+From: Takashi Iwai <tiwai at suse.de>
+Date: Wed, 13 Jan 2016 21:35:06 +0100
+Subject: ALSA: timer: Fix double unlink of active_list
+Origin: https://git.kernel.org/linus/ee8413b01045c74340aa13ad5bdf905de32be736
+
+ALSA timer instance object has a couple of linked lists and they are
+unlinked unconditionally at snd_timer_stop(). Meanwhile
+snd_timer_interrupt() unlinks it, but it calls list_del() which leaves
+the element list itself unchanged. This ends up with unlinking twice,
+and it was caught by syzkaller fuzzer.
+
+The fix is to use list_del_init() variant properly there, too.
+
+Reported-by: Dmitry Vyukov <dvyukov at google.com>
+Tested-by: Dmitry Vyukov <dvyukov at google.com>
+Signed-off-by: Takashi Iwai <tiwai at suse.de>
+Signed-off-by: Ben Hutchings <ben at decadent.org.uk>
+---
+ sound/core/timer.c | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+
+--- a/sound/core/timer.c
++++ b/sound/core/timer.c
+@@ -692,7 +692,7 @@ void snd_timer_interrupt(struct snd_time
+ } else {
+ ti->flags &= ~SNDRV_TIMER_IFLG_RUNNING;
+ if (--timer->running)
+- list_del(&ti->active_list);
++ list_del_init(&ti->active_list);
+ }
+ if ((timer->hw.flags & SNDRV_TIMER_HW_TASKLET) ||
+ (ti->flags & SNDRV_TIMER_IFLG_FAST))
diff --git a/debian/patches/bugfix/all/alsa-timer-fix-race-among-timer-ioctls.patch b/debian/patches/bugfix/all/alsa-timer-fix-race-among-timer-ioctls.patch
new file mode 100644
index 0000000..1ad6589
--- /dev/null
+++ b/debian/patches/bugfix/all/alsa-timer-fix-race-among-timer-ioctls.patch
@@ -0,0 +1,117 @@
+From: Takashi Iwai <tiwai at suse.de>
+Date: Wed, 13 Jan 2016 17:48:01 +0100
+Subject: ALSA: timer: Fix race among timer ioctls
+Origin: https://git.kernel.org/linus/af368027a49a751d6ff4ee9e3f9961f35bb4fede
+
+ALSA timer ioctls have an open race and this may lead to a
+use-after-free of timer instance object. A simplistic fix is to make
+each ioctl exclusive. We have already tread_sem for controlling the
+tread, and extend this as a global mutex to be applied to each ioctl.
+
+The downside is, of course, the worse concurrency. But these ioctls
+aren't to be parallel accessible, in anyway, so it should be fine to
+serialize there.
+
+Reported-by: Dmitry Vyukov <dvyukov at google.com>
+Tested-by: Dmitry Vyukov <dvyukov at google.com>
+Signed-off-by: Takashi Iwai <tiwai at suse.de>
+Signed-off-by: Ben Hutchings <ben at decadent.org.uk>
+---
+ sound/core/timer.c | 32 +++++++++++++++++++-------------
+ 1 file changed, 19 insertions(+), 13 deletions(-)
+
+--- a/sound/core/timer.c
++++ b/sound/core/timer.c
+@@ -72,7 +72,7 @@ struct snd_timer_user {
+ struct timespec tstamp; /* trigger tstamp */
+ wait_queue_head_t qchange_sleep;
+ struct fasync_struct *fasync;
+- struct mutex tread_sem;
++ struct mutex ioctl_lock;
+ };
+
+ /* list of timers */
+@@ -1255,7 +1255,7 @@ static int snd_timer_user_open(struct in
+ return -ENOMEM;
+ spin_lock_init(&tu->qlock);
+ init_waitqueue_head(&tu->qchange_sleep);
+- mutex_init(&tu->tread_sem);
++ mutex_init(&tu->ioctl_lock);
+ tu->ticks = 1;
+ tu->queue_size = 128;
+ tu->queue = kmalloc(tu->queue_size * sizeof(struct snd_timer_read),
+@@ -1275,8 +1275,10 @@ static int snd_timer_user_release(struct
+ if (file->private_data) {
+ tu = file->private_data;
+ file->private_data = NULL;
++ mutex_lock(&tu->ioctl_lock);
+ if (tu->timeri)
+ snd_timer_close(tu->timeri);
++ mutex_unlock(&tu->ioctl_lock);
+ kfree(tu->queue);
+ kfree(tu->tqueue);
+ kfree(tu);
+@@ -1514,7 +1516,6 @@ static int snd_timer_user_tselect(struct
+ int err = 0;
+
+ tu = file->private_data;
+- mutex_lock(&tu->tread_sem);
+ if (tu->timeri) {
+ snd_timer_close(tu->timeri);
+ tu->timeri = NULL;
+@@ -1558,7 +1559,6 @@ static int snd_timer_user_tselect(struct
+ }
+
+ __err:
+- mutex_unlock(&tu->tread_sem);
+ return err;
+ }
+
+@@ -1771,7 +1771,7 @@ enum {
+ SNDRV_TIMER_IOCTL_PAUSE_OLD = _IO('T', 0x23),
+ };
+
+-static long snd_timer_user_ioctl(struct file *file, unsigned int cmd,
++static long __snd_timer_user_ioctl(struct file *file, unsigned int cmd,
+ unsigned long arg)
+ {
+ struct snd_timer_user *tu;
+@@ -1788,17 +1788,11 @@ static long snd_timer_user_ioctl(struct
+ {
+ int xarg;
+
+- mutex_lock(&tu->tread_sem);
+- if (tu->timeri) { /* too late */
+- mutex_unlock(&tu->tread_sem);
++ if (tu->timeri) /* too late */
+ return -EBUSY;
+- }
+- if (get_user(xarg, p)) {
+- mutex_unlock(&tu->tread_sem);
++ if (get_user(xarg, p))
+ return -EFAULT;
+- }
+ tu->tread = xarg ? 1 : 0;
+- mutex_unlock(&tu->tread_sem);
+ return 0;
+ }
+ case SNDRV_TIMER_IOCTL_GINFO:
+@@ -1831,6 +1825,18 @@ static long snd_timer_user_ioctl(struct
+ return -ENOTTY;
+ }
+
++static long snd_timer_user_ioctl(struct file *file, unsigned int cmd,
++ unsigned long arg)
++{
++ struct snd_timer_user *tu = file->private_data;
++ long ret;
++
++ mutex_lock(&tu->ioctl_lock);
++ ret = __snd_timer_user_ioctl(file, cmd, arg);
++ mutex_unlock(&tu->ioctl_lock);
++ return ret;
++}
++
+ static int snd_timer_user_fasync(int fd, struct file * file, int on)
+ {
+ struct snd_timer_user *tu;
diff --git a/debian/patches/bugfix/all/alsa-timer-harden-slave-timer-list-handling.patch b/debian/patches/bugfix/all/alsa-timer-harden-slave-timer-list-handling.patch
new file mode 100644
index 0000000..461f87d
--- /dev/null
+++ b/debian/patches/bugfix/all/alsa-timer-harden-slave-timer-list-handling.patch
@@ -0,0 +1,96 @@
+From: Takashi Iwai <tiwai at suse.de>
+Date: Thu, 14 Jan 2016 16:30:58 +0100
+Subject: ALSA: timer: Harden slave timer list handling
+Origin: https://git.kernel.org/linus/b5a663aa426f4884c71cd8580adae73f33570f0d
+
+A slave timer instance might be still accessible in a racy way while
+operating the master instance as it lacks of locking. Since the
+master operation is mostly protected with timer->lock, we should cope
+with it while changing the slave instance, too. Also, some linked
+lists (active_list and ack_list) of slave instances aren't unlinked
+immediately at stopping or closing, and this may lead to unexpected
+accesses.
+
+This patch tries to address these issues. It adds spin lock of
+timer->lock (either from master or slave, which is equivalent) in a
+few places. For avoiding a deadlock, we ensure that the global
+slave_active_lock is always locked at first before each timer lock.
+
+Also, ack and active_list of slave instances are properly unlinked at
+snd_timer_stop() and snd_timer_close().
+
+Last but not least, remove the superfluous call of _snd_timer_stop()
+at removing slave links. This is a noop, and calling it may confuse
+readers wrt locking. Further cleanup will follow in a later patch.
+
+Actually we've got reports of use-after-free by syzkaller fuzzer, and
+this hopefully fixes these issues.
+
+Reported-by: Dmitry Vyukov <dvyukov at google.com>
+Signed-off-by: Takashi Iwai <tiwai at suse.de>
+Signed-off-by: Ben Hutchings <ben at decadent.org.uk>
+---
+ sound/core/timer.c | 18 ++++++++++++++----
+ 1 file changed, 14 insertions(+), 4 deletions(-)
+
+--- a/sound/core/timer.c
++++ b/sound/core/timer.c
+@@ -214,11 +214,13 @@ static void snd_timer_check_master(struc
+ slave->slave_id == master->slave_id) {
+ list_move_tail(&slave->open_list, &master->slave_list_head);
+ spin_lock_irq(&slave_active_lock);
++ spin_lock(&master->timer->lock);
+ slave->master = master;
+ slave->timer = master->timer;
+ if (slave->flags & SNDRV_TIMER_IFLG_RUNNING)
+ list_add_tail(&slave->active_list,
+ &master->slave_active_head);
++ spin_unlock(&master->timer->lock);
+ spin_unlock_irq(&slave_active_lock);
+ }
+ }
+@@ -344,15 +346,18 @@ int snd_timer_close(struct snd_timer_ins
+ timer->hw.close)
+ timer->hw.close(timer);
+ /* remove slave links */
++ spin_lock_irq(&slave_active_lock);
++ spin_lock(&timer->lock);
+ list_for_each_entry_safe(slave, tmp, &timeri->slave_list_head,
+ open_list) {
+- spin_lock_irq(&slave_active_lock);
+- _snd_timer_stop(slave, 1, SNDRV_TIMER_EVENT_RESOLUTION);
+ list_move_tail(&slave->open_list, &snd_timer_slave_list);
+ slave->master = NULL;
+ slave->timer = NULL;
+- spin_unlock_irq(&slave_active_lock);
++ list_del_init(&slave->ack_list);
++ list_del_init(&slave->active_list);
+ }
++ spin_unlock(&timer->lock);
++ spin_unlock_irq(&slave_active_lock);
+ mutex_unlock(®ister_mutex);
+ }
+ out:
+@@ -439,9 +444,12 @@ static int snd_timer_start_slave(struct
+
+ spin_lock_irqsave(&slave_active_lock, flags);
+ timeri->flags |= SNDRV_TIMER_IFLG_RUNNING;
+- if (timeri->master)
++ if (timeri->master && timeri->timer) {
++ spin_lock(&timeri->timer->lock);
+ list_add_tail(&timeri->active_list,
+ &timeri->master->slave_active_head);
++ spin_unlock(&timeri->timer->lock);
++ }
+ spin_unlock_irqrestore(&slave_active_lock, flags);
+ return 1; /* delayed start */
+ }
+@@ -487,6 +495,8 @@ static int _snd_timer_stop(struct snd_ti
+ if (!keep_flag) {
+ spin_lock_irqsave(&slave_active_lock, flags);
+ timeri->flags &= ~SNDRV_TIMER_IFLG_RUNNING;
++ list_del_init(&timeri->ack_list);
++ list_del_init(&timeri->active_list);
+ spin_unlock_irqrestore(&slave_active_lock, flags);
+ }
+ goto __end;
diff --git a/debian/patches/series b/debian/patches/series
index fdff6fe..3e8088a 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -1207,3 +1207,9 @@ bugfix/all/pipe-fix-buffer-offset-after-partially-failed-read.patch
bugfix/all/alsa-usb-audio-avoid-freeing-umidi-object-twice.patch
bugfix/all/unix-correctly-track-in-flight-fds-in-sending-process-user_struct.patch
bugfix/all/usb-fix-invalid-memory-access-in-hub_activate.patch
+bugfix/all/alsa-seq-fix-missing-null-check-at-remove_events-ioctl.patch
+bugfix/all/alsa-seq-fix-race-at-timer-setup-and-close.patch
+bugfix/all/alsa-timer-fix-double-unlink-of-active_list.patch
+bugfix/all/alsa-timer-fix-race-among-timer-ioctls.patch
+bugfix/all/alsa-timer-harden-slave-timer-list-handling.patch
+bugfix/all/alsa-hrtimer-fix-stall-by-hrtimer_cancel.patch
--
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/kernel/linux.git
More information about the Kernel-svn-changes
mailing list