From 2dbf7df6996e5f0b00131dd1da3ee0e922f8120c Mon Sep 17 00:00:00 2001 From: y00230200 <yanghongliang.yang@huawei.com> Date: Thu, 18 Jan 2018 17:08:24 +0800 Subject: [PATCH] ALSA: seq: Fix use-after-free at creating a port 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. --- sound/core/seq/seq_clientmgr.c | 18 +++++++++++------- sound/core/seq/seq_ports.c | 10 +++++++--- 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c index 225c73152ee9..790456b8762c 100644 --- a/sound/core/seq/seq_clientmgr.c +++ b/sound/core/seq/seq_clientmgr.c @@ -1248,8 +1248,8 @@ static int snd_seq_ioctl_set_client_info(struct snd_seq_client *client, } -/* - * CREATE PORT ioctl() +/* + * CREATE PORT ioctl() */ static int snd_seq_ioctl_create_port(struct snd_seq_client *client, void __user *arg) @@ -1257,6 +1257,7 @@ static int snd_seq_ioctl_create_port(struct snd_seq_client *client, 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; @@ -1270,7 +1271,9 @@ static int snd_seq_ioctl_create_port(struct snd_seq_client *client, 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) { @@ -1292,6 +1295,7 @@ static int snd_seq_ioctl_create_port(struct snd_seq_client *client, 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; @@ -1299,8 +1303,8 @@ static int snd_seq_ioctl_create_port(struct snd_seq_client *client, return 0; } -/* - * DELETE PORT ioctl() +/* + * DELETE PORT ioctl() */ static int snd_seq_ioctl_delete_port(struct snd_seq_client *client, void __user *arg) @@ -1323,8 +1327,8 @@ static int snd_seq_ioctl_delete_port(struct snd_seq_client *client, } -/* - * GET_PORT_INFO ioctl() (on any client) +/* + * GET_PORT_INFO ioctl() (on any client) */ static int snd_seq_ioctl_get_port_info(struct snd_seq_client *client, void __user *arg) diff --git a/sound/core/seq/seq_ports.c b/sound/core/seq/seq_ports.c index 794a341bf0e5..543a1cc8d979 100644 --- a/sound/core/seq/seq_ports.c +++ b/sound/core/seq/seq_ports.c @@ -1,3 +1,4 @@ + /* * ALSA sequencer Ports * Copyright (c) 1998 by Frank van de Pol <fvdpol@coil.demon.nl> @@ -122,14 +123,16 @@ 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) { unsigned long flags; struct snd_seq_client_port *new_port, *p; int num = -1; - + /* sanity check */ if (snd_BUG_ON(!client)) return NULL; @@ -153,6 +156,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); @@ -167,9 +171,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; } -- GitLab