[lkml]   [2017]   [Aug]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
Patch in this message
Subject[PATCH 4.4 07/20] ALSA: seq: 2nd attempt at fixing race creating a queue
4.4-stable review patch.  If anyone has any objections, please let me know.


From: Daniel Mentz <>

commit 7e1d90f60a0d501c8503e636942ca704a454d910 upstream.

commit 4842e98f26dd80be3623c4714a244ba52ea096a8 ("ALSA: seq: Fix race at
creating a queue") attempted to fix a race reported by syzkaller. That
fix has been described as follows:

When a sequencer queue is created in snd_seq_queue_alloc(),it adds the
new queue element to the public list before referencing it. Thus the
queue might be deleted before the call of snd_seq_queue_use(), and it
results in the use-after-free error, as spotted by syzkaller.

The fix is to reference the queue object at the right time.

Even with that fix in place, syzkaller reported a use-after-free error.
It specifically pointed to the last instruction "return q->queue" in
snd_seq_queue_alloc(). The pointer q is being used after kfree() has
been called on it.

It turned out that there is still a small window where a race can
happen. The window opens at
and closes at
snd_seq_ioctl_create_queue()->queueptr()->snd_use_lock_use(). Between
these two calls, a different thread could delete the queue and possibly
re-create a different queue in the same location in queue_list.

This change prevents this situation by calling snd_use_lock_use() from
snd_seq_queue_alloc() prior to calling queue_list_add(). It is then the
caller's responsibility to call snd_use_lock_free(&q->use_lock).

Fixes: 4842e98f26dd ("ALSA: seq: Fix race at creating a queue")
Reported-by: Dmitry Vyukov <>
Signed-off-by: Daniel Mentz <>
Signed-off-by: Takashi Iwai <>
Signed-off-by: Greg Kroah-Hartman <>

sound/core/seq/seq_clientmgr.c | 13 ++++---------
sound/core/seq/seq_queue.c | 14 +++++++++-----
sound/core/seq/seq_queue.h | 2 +-
3 files changed, 14 insertions(+), 15 deletions(-)

--- a/sound/core/seq/seq_clientmgr.c
+++ b/sound/core/seq/seq_clientmgr.c
@@ -1530,19 +1530,14 @@ static int snd_seq_ioctl_create_queue(st
void __user *arg)
struct snd_seq_queue_info info;
- int result;
struct snd_seq_queue *q;

if (copy_from_user(&info, arg, sizeof(info)))
return -EFAULT;

- result = snd_seq_queue_alloc(client->number, info.locked, info.flags);
- if (result < 0)
- return result;
- q = queueptr(result);
- if (q == NULL)
- return -EINVAL;
+ q = snd_seq_queue_alloc(client->number, info.locked, info.flags);
+ if (IS_ERR(q))
+ return PTR_ERR(q);

info.queue = q->queue;
info.locked = q->locked;
@@ -1552,7 +1547,7 @@ static int snd_seq_ioctl_create_queue(st
if (![0])
snprintf(, sizeof(, "Queue-%d", q->queue);
strlcpy(q->name,, sizeof(q->name));
- queuefree(q);
+ snd_use_lock_free(&q->use_lock);

if (copy_to_user(arg, &info, sizeof(info)))
return -EFAULT;
--- a/sound/core/seq/seq_queue.c
+++ b/sound/core/seq/seq_queue.c
@@ -184,22 +184,26 @@ void __exit snd_seq_queues_delete(void)
static void queue_use(struct snd_seq_queue *queue, int client, int use);

/* allocate a new queue -
- * return queue index value or negative value for error
+ * return pointer to new queue or ERR_PTR(-errno) for error
+ * The new queue's use_lock is set to 1. It is the caller's responsibility to
+ * call snd_use_lock_free(&q->use_lock).
-int snd_seq_queue_alloc(int client, int locked, unsigned int info_flags)
+struct snd_seq_queue *snd_seq_queue_alloc(int client, int locked, unsigned int info_flags)
struct snd_seq_queue *q;

q = queue_new(client, locked);
if (q == NULL)
- return -ENOMEM;
+ return ERR_PTR(-ENOMEM);
q->info_flags = info_flags;
queue_use(q, client, 1);
+ snd_use_lock_use(&q->use_lock);
if (queue_list_add(q) < 0) {
+ snd_use_lock_free(&q->use_lock);
- return -ENOMEM;
+ return ERR_PTR(-ENOMEM);
- return q->queue;
+ return q;

/* delete a queue - queue must be owned by the client */
--- a/sound/core/seq/seq_queue.h
+++ b/sound/core/seq/seq_queue.h
@@ -71,7 +71,7 @@ void snd_seq_queues_delete(void);

/* create new queue (constructor) */
-int snd_seq_queue_alloc(int client, int locked, unsigned int flags);
+struct snd_seq_queue *snd_seq_queue_alloc(int client, int locked, unsigned int flags);

/* delete queue (destructor) */
int snd_seq_queue_delete(int client, int queueid);

 \ /
  Last update: 2017-08-22 21:32    [W:0.105 / U:0.456 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site