Re: [PATCH v2] ALSA: seq: 2nd attempt at fixing race creating a q

[Takashi Iwai]

On Mon, 14 Aug 2017 23:46:01 +0200,
Daniel Mentz wrote:
> 
> 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
> snd_seq_ioctl_create_queue()->snd_seq_queue_alloc()->queue_list_add()
> 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).
> 
> Reported-by: Dmitry Vyukov <dvyukov@xxxxxxxxxx>
> Cc: <stable@xxxxxxxxxxxxxxx>
> Cc: Takashi Iwai <tiwai@xxxxxxx>
> Signed-off-by: Daniel Mentz <danielmentz@xxxxxxxxxx>

Applied now.  Thanks!


Takashi