From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Google-Smtp-Source: AG47ELu1GQw8zcmUAyXAoFi5lw4FhylAZTO9zlRZeAuSzbUKN2t7QiFmQIK2kwEzKLEEWGBgP0pE ARC-Seal: i=1; a=rsa-sha256; t=1521214049; cv=none; d=google.com; s=arc-20160816; b=dy78Z0jp1I374fCj8QRHjqS3l4QoIpBQ6YEhmtD4/T0ymiqoK/ti8saH4QtE6F4d4b jIl9Itn8d7GrDHQkF0XLnAqbqC3eRcol3CenFdoq3rbhkO+BJRlY017aKZKZ2ve9HAPF fkTgVwnrBBslCzFwFpVWRKmxVQmxmKE+hPBduScWXSlIgb6EasuIrsN0vBdoglAPts9o 9KSW8ffQohL1RTk3EaX1ctuAhQjdUqGQK+UswSCe90wY2ROh8gvgALVa6YGXqHwpWaZS iaebcAOZUCoWlnCgp1wNOcJfM52k+wwCq8vSq7+6CCQaye+XGIsci07Hvg7ehBH+5sTf BSRw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=mime-version:user-agent:references:in-reply-to:message-id:date :subject:cc:to:from:arc-authentication-results; bh=X94hNI5F/1GVztmWZ6ahYTYsXH+1j90f3T6xDC3hhRk=; b=MsVa/kW+XoyrncHSZBBXjW4FUy1hSgZe3SqEtbrWj6Pgqhl3J7QJwI99hOfqTV/mBI iBu9db8vqOeYxo8GyO2gLnCxx8ONvmzr9O4BgVXfE/sN+UOcqlD2SlUQST06MkkaMGkA BdRfArDjiqddGBBWbefwdNRXWt1KzhPVT5h4oEldqeZ/hvLtwLEdBTeXkkdzqpIG6epq hyUW0BM4GmBTGFVKbqXgag4ux4jQ/0+hptE8VXj7JhQ1x5OFB4KItGdm31YpuP4dp77w 5h1rFniYV+yhP0Ni0CrUQSl8iOCapRJMo4XRf3DvCSMeWlGBZyJuTd1uF5bVgc4nQCEU EQvQ== ARC-Authentication-Results: i=1; mx.google.com; spf=softfail (google.com: domain of transitioning gregkh@linuxfoundation.org does not designate 90.92.61.202 as permitted sender) smtp.mailfrom=gregkh@linuxfoundation.org Authentication-Results: mx.google.com; spf=softfail (google.com: domain of transitioning gregkh@linuxfoundation.org does not designate 90.92.61.202 as permitted sender) smtp.mailfrom=gregkh@linuxfoundation.org From: Greg Kroah-Hartman To: linux-kernel@vger.kernel.org Cc: Greg Kroah-Hartman , stable@vger.kernel.org, =?UTF-8?q?=E8=8C=83=E9=BE=99=E9=A3=9E?= , Nicolai Stange , Takashi Iwai Subject: [PATCH 4.4 23/63] ALSA: seq: More protection for concurrent write and ioctl races Date: Fri, 16 Mar 2018 16:22:55 +0100 Message-Id: <20180316152302.687787272@linuxfoundation.org> X-Mailer: git-send-email 2.16.2 In-Reply-To: <20180316152259.964532775@linuxfoundation.org> References: <20180316152259.964532775@linuxfoundation.org> User-Agent: quilt/0.65 X-stable: review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-LABELS: =?utf-8?b?IlxcU2VudCI=?= X-GMAIL-THRID: =?utf-8?q?1595108299103257252?= X-GMAIL-MSGID: =?utf-8?q?1595108542553883580?= X-Mailing-List: linux-kernel@vger.kernel.org List-ID: 4.4-stable review patch. If anyone has any objections, please let me know. ------------------ From: Takashi Iwai commit 7bd80091567789f1c0cb70eb4737aac8bcd2b6b9 upstream. This patch is an attempt for further hardening against races between the concurrent write and ioctls. The previous fix d15d662e89fc ("ALSA: seq: Fix racy pool initializations") covered the race of the pool initialization at writer and the pool resize ioctl by the client->ioctl_mutex (CVE-2018-1000004). However, basically this mutex should be applied more widely to the whole write operation for avoiding the unexpected pool operations by another thread. The only change outside snd_seq_write() is the additional mutex argument to helper functions, so that we can unlock / relock the given mutex temporarily during schedule() call for blocking write. Fixes: d15d662e89fc ("ALSA: seq: Fix racy pool initializations") Reported-by: 范龙飞 Reported-by: Nicolai Stange Reviewed-and-tested-by: Nicolai Stange Cc: Signed-off-by: Takashi Iwai Signed-off-by: Greg Kroah-Hartman --- sound/core/seq/seq_clientmgr.c | 18 +++++++++++------- sound/core/seq/seq_fifo.c | 2 +- sound/core/seq/seq_memory.c | 14 ++++++++++---- sound/core/seq/seq_memory.h | 3 ++- 4 files changed, 24 insertions(+), 13 deletions(-) --- a/sound/core/seq/seq_clientmgr.c +++ b/sound/core/seq/seq_clientmgr.c @@ -919,7 +919,8 @@ int snd_seq_dispatch_event(struct snd_se static int snd_seq_client_enqueue_event(struct snd_seq_client *client, struct snd_seq_event *event, struct file *file, int blocking, - int atomic, int hop) + int atomic, int hop, + struct mutex *mutexp) { struct snd_seq_event_cell *cell; int err; @@ -957,7 +958,8 @@ static int snd_seq_client_enqueue_event( return -ENXIO; /* queue is not allocated */ /* allocate an event cell */ - err = snd_seq_event_dup(client->pool, event, &cell, !blocking || atomic, file); + err = snd_seq_event_dup(client->pool, event, &cell, !blocking || atomic, + file, mutexp); if (err < 0) return err; @@ -1026,12 +1028,11 @@ static ssize_t snd_seq_write(struct file return -ENXIO; /* allocate the pool now if the pool is not allocated yet */ + mutex_lock(&client->ioctl_mutex); if (client->pool->size > 0 && !snd_seq_write_pool_allocated(client)) { - mutex_lock(&client->ioctl_mutex); err = snd_seq_pool_init(client->pool); - mutex_unlock(&client->ioctl_mutex); if (err < 0) - return -ENOMEM; + goto out; } /* only process whole events */ @@ -1082,7 +1083,7 @@ static ssize_t snd_seq_write(struct file /* ok, enqueue it */ err = snd_seq_client_enqueue_event(client, &event, file, !(file->f_flags & O_NONBLOCK), - 0, 0); + 0, 0, &client->ioctl_mutex); if (err < 0) break; @@ -1093,6 +1094,8 @@ static ssize_t snd_seq_write(struct file written += len; } + out: + mutex_unlock(&client->ioctl_mutex); return written ? written : err; } @@ -2350,7 +2353,8 @@ static int kernel_client_enqueue(int cli if (! cptr->accept_output) result = -EPERM; else /* send it */ - result = snd_seq_client_enqueue_event(cptr, ev, file, blocking, atomic, hop); + result = snd_seq_client_enqueue_event(cptr, ev, file, blocking, + atomic, hop, NULL); snd_seq_client_unlock(cptr); return result; --- a/sound/core/seq/seq_fifo.c +++ b/sound/core/seq/seq_fifo.c @@ -123,7 +123,7 @@ int snd_seq_fifo_event_in(struct snd_seq return -EINVAL; snd_use_lock_use(&f->use_lock); - err = snd_seq_event_dup(f->pool, event, &cell, 1, NULL); /* always non-blocking */ + err = snd_seq_event_dup(f->pool, event, &cell, 1, NULL, NULL); /* always non-blocking */ if (err < 0) { if ((err == -ENOMEM) || (err == -EAGAIN)) atomic_inc(&f->overflow); --- a/sound/core/seq/seq_memory.c +++ b/sound/core/seq/seq_memory.c @@ -221,7 +221,8 @@ void snd_seq_cell_free(struct snd_seq_ev */ static int snd_seq_cell_alloc(struct snd_seq_pool *pool, struct snd_seq_event_cell **cellp, - int nonblock, struct file *file) + int nonblock, struct file *file, + struct mutex *mutexp) { struct snd_seq_event_cell *cell; unsigned long flags; @@ -245,7 +246,11 @@ static int snd_seq_cell_alloc(struct snd set_current_state(TASK_INTERRUPTIBLE); add_wait_queue(&pool->output_sleep, &wait); spin_unlock_irq(&pool->lock); + if (mutexp) + mutex_unlock(mutexp); schedule(); + if (mutexp) + mutex_lock(mutexp); spin_lock_irq(&pool->lock); remove_wait_queue(&pool->output_sleep, &wait); /* interrupted? */ @@ -288,7 +293,7 @@ __error: */ int snd_seq_event_dup(struct snd_seq_pool *pool, struct snd_seq_event *event, struct snd_seq_event_cell **cellp, int nonblock, - struct file *file) + struct file *file, struct mutex *mutexp) { int ncells, err; unsigned int extlen; @@ -305,7 +310,7 @@ int snd_seq_event_dup(struct snd_seq_poo if (ncells >= pool->total_elements) return -ENOMEM; - err = snd_seq_cell_alloc(pool, &cell, nonblock, file); + err = snd_seq_cell_alloc(pool, &cell, nonblock, file, mutexp); if (err < 0) return err; @@ -331,7 +336,8 @@ int snd_seq_event_dup(struct snd_seq_poo int size = sizeof(struct snd_seq_event); if (len < size) size = len; - err = snd_seq_cell_alloc(pool, &tmp, nonblock, file); + err = snd_seq_cell_alloc(pool, &tmp, nonblock, file, + mutexp); if (err < 0) goto __error; if (cell->event.data.ext.ptr == NULL) --- a/sound/core/seq/seq_memory.h +++ b/sound/core/seq/seq_memory.h @@ -66,7 +66,8 @@ struct snd_seq_pool { void snd_seq_cell_free(struct snd_seq_event_cell *cell); int snd_seq_event_dup(struct snd_seq_pool *pool, struct snd_seq_event *event, - struct snd_seq_event_cell **cellp, int nonblock, struct file *file); + struct snd_seq_event_cell **cellp, int nonblock, + struct file *file, struct mutex *mutexp); /* return number of unused (free) cells */ static inline int snd_seq_unused_cells(struct snd_seq_pool *pool)