Skip to content

Commit

Permalink
ALSA: seq: Fix racy cell insertions during snd_seq_pool_done()
Browse files Browse the repository at this point in the history
When snd_seq_pool_done() is called, it marks the closing flag to
refuse the further cell insertions.  But snd_seq_pool_done() itself
doesn't clear the cells but just waits until all cells are cleared by
the caller side.  That is, it's racy, and this leads to the endless
stall as syzkaller spotted.

This patch addresses the racy by splitting the setup of pool->closing
flag out of snd_seq_pool_done(), and calling it properly before
snd_seq_pool_done().

BugLink: http://lkml.kernel.org/r/CACT4Y+aqqy8bZA1fFieifNxR2fAfFQQABcBHj801+u5ePV0URw@mail.gmail.com
Reported-and-tested-by: Dmitry Vyukov <dvyukov@google.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
  • Loading branch information
Takashi Iwai committed Mar 21, 2017
1 parent c6736a9 commit c520ff3
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 4 deletions.
1 change: 1 addition & 0 deletions sound/core/seq/seq_clientmgr.c
Original file line number Diff line number Diff line change
Expand Up @@ -1832,6 +1832,7 @@ static int snd_seq_ioctl_set_client_pool(struct snd_seq_client *client,
info->output_pool != client->pool->size)) {
if (snd_seq_write_pool_allocated(client)) {
/* remove all existing cells */
snd_seq_pool_mark_closing(client->pool);
snd_seq_queue_client_leave_cells(client->number);
snd_seq_pool_done(client->pool);
}
Expand Down
3 changes: 3 additions & 0 deletions sound/core/seq/seq_fifo.c
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,9 @@ void snd_seq_fifo_delete(struct snd_seq_fifo **fifo)
return;
*fifo = NULL;

if (f->pool)
snd_seq_pool_mark_closing(f->pool);

snd_seq_fifo_clear(f);

/* wake up clients if any */
Expand Down
17 changes: 13 additions & 4 deletions sound/core/seq/seq_memory.c
Original file line number Diff line number Diff line change
Expand Up @@ -415,6 +415,18 @@ int snd_seq_pool_init(struct snd_seq_pool *pool)
return 0;
}

/* refuse the further insertion to the pool */
void snd_seq_pool_mark_closing(struct snd_seq_pool *pool)
{
unsigned long flags;

if (snd_BUG_ON(!pool))
return;
spin_lock_irqsave(&pool->lock, flags);
pool->closing = 1;
spin_unlock_irqrestore(&pool->lock, flags);
}

/* remove events */
int snd_seq_pool_done(struct snd_seq_pool *pool)
{
Expand All @@ -425,10 +437,6 @@ int snd_seq_pool_done(struct snd_seq_pool *pool)
return -EINVAL;

/* wait for closing all threads */
spin_lock_irqsave(&pool->lock, flags);
pool->closing = 1;
spin_unlock_irqrestore(&pool->lock, flags);

if (waitqueue_active(&pool->output_sleep))
wake_up(&pool->output_sleep);

Expand Down Expand Up @@ -485,6 +493,7 @@ int snd_seq_pool_delete(struct snd_seq_pool **ppool)
*ppool = NULL;
if (pool == NULL)
return 0;
snd_seq_pool_mark_closing(pool);
snd_seq_pool_done(pool);
kfree(pool);
return 0;
Expand Down
1 change: 1 addition & 0 deletions sound/core/seq/seq_memory.h
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ static inline int snd_seq_total_cells(struct snd_seq_pool *pool)
int snd_seq_pool_init(struct snd_seq_pool *pool);

/* done pool - free events */
void snd_seq_pool_mark_closing(struct snd_seq_pool *pool);
int snd_seq_pool_done(struct snd_seq_pool *pool);

/* create pool */
Expand Down

0 comments on commit c520ff3

Please sign in to comment.