From 52c8b81bca6bdf1330fb8b49e5f387f6e59cf3ac Mon Sep 17 00:00:00 2001 From: Salvatore Bonaccorso Date: Fri, 13 Oct 2017 06:51:57 +0200 Subject: [PATCH] ALSA: seq: Fix use-after-free at creating a port (CVE-2017-15265) --- debian/changelog | 1 + ...ix-use-after-free-at-creating-a-port.patch | 141 ++++++++++++++++++ debian/patches/series | 1 + 3 files changed, 143 insertions(+) create mode 100644 debian/patches/bugfix/all/ALSA-seq-Fix-use-after-free-at-creating-a-port.patch diff --git a/debian/changelog b/debian/changelog index 807c84bcc..e1615d5ef 100644 --- a/debian/changelog +++ b/debian/changelog @@ -13,6 +13,7 @@ linux (4.13.4-2) UNRELEASED; urgency=medium Thanks to Eric Côté (Closes: #878092) * KEYS: prevent KEYCTL_READ on negative key (CVE-2017-12192) * waitid(): Add missing access_ok() checks (CVE-2017-5123) + * ALSA: seq: Fix use-after-free at creating a port (CVE-2017-15265) -- Ben Hutchings Wed, 04 Oct 2017 23:14:54 +0100 diff --git a/debian/patches/bugfix/all/ALSA-seq-Fix-use-after-free-at-creating-a-port.patch b/debian/patches/bugfix/all/ALSA-seq-Fix-use-after-free-at-creating-a-port.patch new file mode 100644 index 000000000..f9026ce22 --- /dev/null +++ b/debian/patches/bugfix/all/ALSA-seq-Fix-use-after-free-at-creating-a-port.patch @@ -0,0 +1,141 @@ +From: Takashi Iwai +Date: Mon, 9 Oct 2017 11:09:20 +0200 +Subject: ALSA: seq: Fix use-after-free at creating a port +Origin: https://git.kernel.org/linus/71105998845fb012937332fe2e806d443c09e026 +Bug-Debian-Security: https://security-tracker.debian.org/tracker/CVE-2017-15265 + +There is a potential race window opened at creating and deleting a +port via ioctl, as spotted by fuzzing. snd_seq_create_port() creates +a port object and returns its pointer, but it doesn't take the +refcount, thus it can be deleted immediately by another thread. +Meanwhile, snd_seq_ioctl_create_port() still calls the function +snd_seq_system_client_ev_port_start() with the created port object +that is being deleted, and this triggers use-after-free like: + + BUG: KASAN: use-after-free in snd_seq_ioctl_create_port+0x504/0x630 [snd_seq] at addr ffff8801f2241cb1 + ============================================================================= + BUG kmalloc-512 (Tainted: G B ): kasan: bad access detected + ----------------------------------------------------------------------------- + INFO: Allocated in snd_seq_create_port+0x94/0x9b0 [snd_seq] age=1 cpu=3 pid=4511 + ___slab_alloc+0x425/0x460 + __slab_alloc+0x20/0x40 + kmem_cache_alloc_trace+0x150/0x190 + snd_seq_create_port+0x94/0x9b0 [snd_seq] + snd_seq_ioctl_create_port+0xd1/0x630 [snd_seq] + snd_seq_do_ioctl+0x11c/0x190 [snd_seq] + snd_seq_ioctl+0x40/0x80 [snd_seq] + do_vfs_ioctl+0x54b/0xda0 + SyS_ioctl+0x79/0x90 + entry_SYSCALL_64_fastpath+0x16/0x75 + INFO: Freed in port_delete+0x136/0x1a0 [snd_seq] age=1 cpu=2 pid=4717 + __slab_free+0x204/0x310 + kfree+0x15f/0x180 + port_delete+0x136/0x1a0 [snd_seq] + snd_seq_delete_port+0x235/0x350 [snd_seq] + snd_seq_ioctl_delete_port+0xc8/0x180 [snd_seq] + snd_seq_do_ioctl+0x11c/0x190 [snd_seq] + snd_seq_ioctl+0x40/0x80 [snd_seq] + do_vfs_ioctl+0x54b/0xda0 + SyS_ioctl+0x79/0x90 + entry_SYSCALL_64_fastpath+0x16/0x75 + Call Trace: + [] dump_stack+0x63/0x82 + [] print_trailer+0xfb/0x160 + [] object_err+0x34/0x40 + [] kasan_report.part.2+0x223/0x520 + [] ? snd_seq_ioctl_create_port+0x504/0x630 [snd_seq] + [] __asan_report_load1_noabort+0x2e/0x30 + [] snd_seq_ioctl_create_port+0x504/0x630 [snd_seq] + [] ? snd_seq_ioctl_delete_port+0x180/0x180 [snd_seq] + [] ? taskstats_exit+0xbc0/0xbc0 + [] snd_seq_do_ioctl+0x11c/0x190 [snd_seq] + [] snd_seq_ioctl+0x40/0x80 [snd_seq] + [] ? acct_account_cputime+0x63/0x80 + [] do_vfs_ioctl+0x54b/0xda0 + ..... + +We may fix this in a few different ways, and in this patch, it's fixed +simply by taking the refcount properly at snd_seq_create_port() and +letting the caller unref the object after use. Also, there is another +potential use-after-free by sprintf() call in snd_seq_create_port(), +and this is moved inside the lock. + +This fix covers CVE-2017-15265. + +Reported-and-tested-by: Michael23 Yu +Suggested-by: Linus Torvalds +Cc: +Signed-off-by: Takashi Iwai +--- + sound/core/seq/seq_clientmgr.c | 6 +++++- + sound/core/seq/seq_ports.c | 7 +++++-- + 2 files changed, 10 insertions(+), 3 deletions(-) + +diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c +index ea2d0ae85bd3..6c9cba2166d9 100644 +--- a/sound/core/seq/seq_clientmgr.c ++++ b/sound/core/seq/seq_clientmgr.c +@@ -1259,6 +1259,7 @@ static int snd_seq_ioctl_create_port(struct snd_seq_client *client, void *arg) + struct snd_seq_port_info *info = arg; + struct snd_seq_client_port *port; + struct snd_seq_port_callback *callback; ++ int port_idx; + + /* it is not allowed to create the port for an another client */ + if (info->addr.client != client->number) +@@ -1269,7 +1270,9 @@ static int snd_seq_ioctl_create_port(struct snd_seq_client *client, void *arg) + return -ENOMEM; + + if (client->type == USER_CLIENT && info->kernel) { +- snd_seq_delete_port(client, port->addr.port); ++ port_idx = port->addr.port; ++ snd_seq_port_unlock(port); ++ snd_seq_delete_port(client, port_idx); + return -EINVAL; + } + if (client->type == KERNEL_CLIENT) { +@@ -1290,6 +1293,7 @@ static int snd_seq_ioctl_create_port(struct snd_seq_client *client, void *arg) + + snd_seq_set_port_info(port, info); + snd_seq_system_client_ev_port_start(port->addr.client, port->addr.port); ++ snd_seq_port_unlock(port); + + return 0; + } +diff --git a/sound/core/seq/seq_ports.c b/sound/core/seq/seq_ports.c +index 0a7020c82bfc..d21ece9f8d73 100644 +--- a/sound/core/seq/seq_ports.c ++++ b/sound/core/seq/seq_ports.c +@@ -122,7 +122,9 @@ static void port_subs_info_init(struct snd_seq_port_subs_info *grp) + } + + +-/* create a port, port number is returned (-1 on failure) */ ++/* create a port, port number is returned (-1 on failure); ++ * the caller needs to unref the port via snd_seq_port_unlock() appropriately ++ */ + struct snd_seq_client_port *snd_seq_create_port(struct snd_seq_client *client, + int port) + { +@@ -151,6 +153,7 @@ struct snd_seq_client_port *snd_seq_create_port(struct snd_seq_client *client, + snd_use_lock_init(&new_port->use_lock); + port_subs_info_init(&new_port->c_src); + port_subs_info_init(&new_port->c_dest); ++ snd_use_lock_use(&new_port->use_lock); + + num = port >= 0 ? port : 0; + mutex_lock(&client->ports_mutex); +@@ -165,9 +168,9 @@ struct snd_seq_client_port *snd_seq_create_port(struct snd_seq_client *client, + list_add_tail(&new_port->list, &p->list); + client->num_ports++; + new_port->addr.port = num; /* store the port number in the port */ ++ sprintf(new_port->name, "port-%d", num); + write_unlock_irqrestore(&client->ports_lock, flags); + mutex_unlock(&client->ports_mutex); +- sprintf(new_port->name, "port-%d", num); + + return new_port; + } +-- +2.11.0 + diff --git a/debian/patches/series b/debian/patches/series index 966296612..0131f69c4 100644 --- a/debian/patches/series +++ b/debian/patches/series @@ -125,6 +125,7 @@ bugfix/all/powerpc-64s-Use-emergency-stack-for-kernel-TM-Bad-Th.patch bugfix/all/powerpc-tm-Fix-illegal-TM-state-in-signal-handler.patch bugfix/all/KEYS-prevent-KEYCTL_READ-on-negative-key.patch bugfix/all/waitid-Add-missing-access_ok-checks.patch +bugfix/all/ALSA-seq-Fix-use-after-free-at-creating-a-port.patch # Fix exported symbol versions bugfix/alpha/alpha-restore-symbol-versions-for-symbols-exported-f.patch