Skip to content

Commit 7110599

Browse files
committed
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>
1 parent 124751d commit 7110599

File tree

2 files changed

+10
-3
lines changed

2 files changed

+10
-3
lines changed

Diff for: sound/core/seq/seq_clientmgr.c

+5-1
Original file line numberDiff line numberDiff line change
@@ -1259,6 +1259,7 @@ static int snd_seq_ioctl_create_port(struct snd_seq_client *client, void *arg)
12591259
struct snd_seq_port_info *info = arg;
12601260
struct snd_seq_client_port *port;
12611261
struct snd_seq_port_callback *callback;
1262+
int port_idx;
12621263

12631264
/* it is not allowed to create the port for an another client */
12641265
if (info->addr.client != client->number)
@@ -1269,7 +1270,9 @@ static int snd_seq_ioctl_create_port(struct snd_seq_client *client, void *arg)
12691270
return -ENOMEM;
12701271

12711272
if (client->type == USER_CLIENT && info->kernel) {
1272-
snd_seq_delete_port(client, port->addr.port);
1273+
port_idx = port->addr.port;
1274+
snd_seq_port_unlock(port);
1275+
snd_seq_delete_port(client, port_idx);
12731276
return -EINVAL;
12741277
}
12751278
if (client->type == KERNEL_CLIENT) {
@@ -1290,6 +1293,7 @@ static int snd_seq_ioctl_create_port(struct snd_seq_client *client, void *arg)
12901293

12911294
snd_seq_set_port_info(port, info);
12921295
snd_seq_system_client_ev_port_start(port->addr.client, port->addr.port);
1296+
snd_seq_port_unlock(port);
12931297

12941298
return 0;
12951299
}

Diff for: sound/core/seq/seq_ports.c

+5-2
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,9 @@ static void port_subs_info_init(struct snd_seq_port_subs_info *grp)
122122
}
123123

124124

125-
/* create a port, port number is returned (-1 on failure) */
125+
/* create a port, port number is returned (-1 on failure);
126+
* the caller needs to unref the port via snd_seq_port_unlock() appropriately
127+
*/
126128
struct snd_seq_client_port *snd_seq_create_port(struct snd_seq_client *client,
127129
int port)
128130
{
@@ -151,6 +153,7 @@ struct snd_seq_client_port *snd_seq_create_port(struct snd_seq_client *client,
151153
snd_use_lock_init(&new_port->use_lock);
152154
port_subs_info_init(&new_port->c_src);
153155
port_subs_info_init(&new_port->c_dest);
156+
snd_use_lock_use(&new_port->use_lock);
154157

155158
num = port >= 0 ? port : 0;
156159
mutex_lock(&client->ports_mutex);
@@ -165,9 +168,9 @@ struct snd_seq_client_port *snd_seq_create_port(struct snd_seq_client *client,
165168
list_add_tail(&new_port->list, &p->list);
166169
client->num_ports++;
167170
new_port->addr.port = num; /* store the port number in the port */
171+
sprintf(new_port->name, "port-%d", num);
168172
write_unlock_irqrestore(&client->ports_lock, flags);
169173
mutex_unlock(&client->ports_mutex);
170-
sprintf(new_port->name, "port-%d", num);
171174

172175
return new_port;
173176
}

0 commit comments

Comments
 (0)