On Thu, 08 Mar 2018 11:44:47 +0100,
Nicolai Stange wrote:
>
> 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"))?
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
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
