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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <s5ha831jrjy.wl-tiwai@suse.de>
Date:   Tue, 15 Aug 2017 08:03:45 +0200
From:   Takashi Iwai <tiwai@...e.de>
To:     Daniel Mentz <danielmentz@...gle.com>
Cc:     alsa-devel@...a-project.org, linux-kernel@...r.kernel.org,
        perex@...ex.cz, stable@...r.kernel.org
Subject: Re: [PATCH v2] ALSA: seq: 2nd attempt at fixing race creating a q

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@...gle.com>
> Cc: <stable@...r.kernel.org>
> Cc: Takashi Iwai <tiwai@...e.de>
> Signed-off-by: Daniel Mentz <danielmentz@...gle.com>

Applied now.  Thanks!


Takashi