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.
diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c
index 225c7315..790456b 100644
--- a/sound/core/seq/seq_clientmgr.c
+++ b/sound/core/seq/seq_clientmgr.c
@@ -1248,8 +1248,8 @@
}
-/*
- * 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 @@
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 @@
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 @@
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 @@
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 @@
}
-/*
- * 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 794a341..543a1cc 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 @@
}
-/* 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 @@
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 @@
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;
}
