CVE-2017-15265: Concurrent Execution using Shared Resource with Improper Synchronization ('Race Condition') (CWE-362)
value: Race condition in the ALSA subsystem in the Linux kernel before 4.13.8 allows local users to cause a denial of service (use-after-free) or possibly have unspecified other impact via crafted /dev/snd/seq ioctl calls, related to sound/core/seq/seq_clientmgr.c and sound/core/seq/seq_ports.c.
CWE description: The product contains a code sequence that can run concurrently with other code, and the code sequence requires temporary, exclusive access to a shared resource, but a timing window exists in which the shared resource can be modified by another code sequence that is operating concurrently.
CVSS v3 base score: 7
Published: 2017-10-16T18:29Z
Repository: https://github.com/torvalds/linux
Language: C
Files changed: sound/core/seq/seq_clientmgr.c, sound/core/seq/seq_ports.c
Commit date: 2017-10-11 09:58:18 +0200
Commit message: ALSA: seq: Fix use-after-free at creating a port

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>
Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>

Diff: diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c
index ea2d0ae85bd367..6c9cba2166d95b 100644
--- a/sound/core/seq/seq_clientmgr.c
+++ b/sound/core/seq/seq_clientmgr.c
@@ -1257,21 +1257,24 @@ static int snd_seq_ioctl_set_client_info(struct snd_seq_client *client,
 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)
 		return -EPERM;
 
 	port = snd_seq_create_port(client, (info->flags & SNDRV_SEQ_PORT_FLG_GIVEN_PORT) ? info->addr.port : -1);
 	if (port == NULL)
 		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) {
 		if ((callback = info->kernel) != NULL) {
 			if (callback->owner)
@@ -1288,10 +1291,11 @@ static int snd_seq_ioctl_create_port(struct snd_seq_client *client, void *arg)
 
 	info->addr = port->addr;
 
 	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 0a7020c82bfc76..d21ece9f8d7365 100644
--- a/sound/core/seq/seq_ports.c
+++ b/sound/core/seq/seq_ports.c
@@ -120,11 +120,13 @@ static void port_subs_info_init(struct snd_seq_port_subs_info *grp)
 	grp->open = NULL;
 	grp->close = NULL;
 }
 
 
-/* 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;
@@ -149,10 +151,11 @@ struct snd_seq_client_port *snd_seq_create_port(struct snd_seq_client *client,
 	new_port->owner = THIS_MODULE;
 	sprintf(new_port->name, "port-%d", num);
 	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);
 	write_lock_irqsave(&client->ports_lock, flags);
 	list_for_each_entry(p, &client->ports_list_head, list) {
@@ -163,13 +166,13 @@ struct snd_seq_client_port *snd_seq_create_port(struct snd_seq_client *client,
 	}
 	/* insert the new port */
 	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;
 }
 
 /* */

Vulnerable code: 		snd_seq_delete_port(client, port->addr.port);
/* create a port, port number is returned (-1 on failure) */
	sprintf(new_port->name, "port-%d", num);
Fixed code: 	int port_idx;
		port_idx = port->addr.port;
		snd_seq_port_unlock(port);
		snd_seq_delete_port(client, port_idx);
	snd_seq_port_unlock(port);
/* create a port, port number is returned (-1 on failure);
 * the caller needs to unref the port via snd_seq_port_unlock() appropriately
 */
	snd_use_lock_use(&new_port->use_lock);
	sprintf(new_port->name, "port-%d", num);