• Daniel Mentz's avatar
    ALSA: seq: 2nd attempt at fixing race creating a queue · bafb25c5
    Daniel Mentz authored
    commit 7e1d90f6 upstream.
    
    commit 4842e98f ("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).
    
    Fixes: 4842e98f ("ALSA: seq: Fix race at creating a queue")
    Reported-by: default avatarDmitry Vyukov <dvyukov@google.com>
    Signed-off-by: default avatarDaniel Mentz <danielmentz@google.com>
    Signed-off-by: default avatarTakashi Iwai <tiwai@suse.de>
    Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
    bafb25c5
seq_clientmgr.c 65.2 KB