Takashi Iwai <tiwai@xxxxxxx> 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@xxxxxxx>
> Reported-by: Nicolai Stange <nstange@xxxxxxx>
> Cc: <stable@xxxxxxxxxxxxxxx>
> Signed-off-by: Takashi Iwai <tiwai@xxxxxxx>
> ---
> 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"))?
Thanks,
Nicolai
--
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton,
HRB 21284 (AG Nürnberg)
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
