| From: Takashi Iwai <tiwai@suse.de> |
| Date: Mon, 9 Oct 2017 11:09:20 +0200 |
| Subject: ALSA: seq: Fix use-after-free at creating a port |
| |
| commit 71105998845fb012937332fe2e806d443c09e026 upstream. |
| |
| 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: |
| [<ffffffff81b03781>] dump_stack+0x63/0x82 |
| [<ffffffff81531b3b>] print_trailer+0xfb/0x160 |
| [<ffffffff81536db4>] object_err+0x34/0x40 |
| [<ffffffff815392d3>] kasan_report.part.2+0x223/0x520 |
| [<ffffffffa07aadf4>] ? snd_seq_ioctl_create_port+0x504/0x630 [snd_seq] |
| [<ffffffff815395fe>] __asan_report_load1_noabort+0x2e/0x30 |
| [<ffffffffa07aadf4>] snd_seq_ioctl_create_port+0x504/0x630 [snd_seq] |
| [<ffffffffa07aa8f0>] ? snd_seq_ioctl_delete_port+0x180/0x180 [snd_seq] |
| [<ffffffff8136be50>] ? taskstats_exit+0xbc0/0xbc0 |
| [<ffffffffa07abc5c>] snd_seq_do_ioctl+0x11c/0x190 [snd_seq] |
| [<ffffffffa07abd10>] snd_seq_ioctl+0x40/0x80 [snd_seq] |
| [<ffffffff8136d433>] ? acct_account_cputime+0x63/0x80 |
| [<ffffffff815b515b>] 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 <ycqzsy@gmail.com> |
| Suggested-by: Linus Torvalds <torvalds@linux-foundation.org> |
| Signed-off-by: Takashi Iwai <tiwai@suse.de> |
| [bwh: Backported to 3.2: adjust context] |
| Signed-off-by: Ben Hutchings <ben@decadent.org.uk> |
| --- |
| sound/core/seq/seq_clientmgr.c | 6 +++++- |
| sound/core/seq/seq_ports.c | 7 +++++-- |
| 2 files changed, 10 insertions(+), 3 deletions(-) |
| |
| --- a/sound/core/seq/seq_clientmgr.c |
| +++ b/sound/core/seq/seq_clientmgr.c |
| @@ -1248,6 +1248,7 @@ static int snd_seq_ioctl_create_port(str |
| struct snd_seq_client_port *port; |
| struct snd_seq_port_info info; |
| struct snd_seq_port_callback *callback; |
| + int port_idx; |
| |
| if (copy_from_user(&info, arg, sizeof(info))) |
| return -EFAULT; |
| @@ -1261,7 +1262,9 @@ static int snd_seq_ioctl_create_port(str |
| 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) { |
| @@ -1283,6 +1286,7 @@ static int snd_seq_ioctl_create_port(str |
| |
| 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); |
| |
| if (copy_to_user(arg, &info, sizeof(info))) |
| return -EFAULT; |
| --- 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 s |
| } |
| |
| |
| -/* 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) |
| { |
| @@ -153,6 +155,7 @@ struct snd_seq_client_port *snd_seq_crea |
| 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); |
| @@ -167,9 +170,9 @@ struct snd_seq_client_port *snd_seq_crea |
| 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; |
| } |