On Thu, 08 Mar 2018 11:44:47 +0100, Nicolai Stange wrote:
Takashi Iwai tiwai@suse.de writes:
This is a fix for a (sort of) fallout in the recent commit d15d662e89fc ("ALSA: seq: Fix racy pool initializations") for CVE-2018-1000004. As the pool resize deletes the existing cells, it may lead to a race when another thread is writing concurrently, eventually resulting a UAF.
A simple workaround is not to allow the pool resizing when the pool is in use. It's an invalid behavior in anyway.
Fixes: d15d662e89fc ("ALSA: seq: Fix racy pool initializations") Reported-by: 范龙飞 long7573@126.com Reported-by: Nicolai Stange nstange@suse.de Cc: stable@vger.kernel.org Signed-off-by: Takashi Iwai tiwai@suse.de
sound/core/seq/seq_clientmgr.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c index 04d4db44fae5..d41ce3ed62ca 100644 --- a/sound/core/seq/seq_clientmgr.c +++ b/sound/core/seq/seq_clientmgr.c @@ -1838,6 +1838,9 @@ static int snd_seq_ioctl_set_client_pool(struct snd_seq_client *client, (! snd_seq_write_pool_allocated(client) || info->output_pool != client->pool->size)) { if (snd_seq_write_pool_allocated(client)) {
Maybe I'm missing something, but doesn't this
/* is the pool in use? */if (atomic_read(&client->pool->counter))return -EBUSY;/* remove all existing cells */ snd_seq_pool_mark_closing(client->pool);render this
snd_seq_queue_client_leave_cells(client->number);useless (assuming the presence of [2/2] ("ALSA: seq: More protection for concurrent write and ioctl races"))?
Right, after the second patch, this call can be removed, indeed. I'll cook up the third patch to remove the superfluous call.
thanks,
Takashi