ALSA: seq: More protection for concurrent write and ioctl races

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: 范龙飞 <long7573@126.com>
Reported-by: Nicolai Stange <nstange@suse.de>
Reviewed-and-tested-by: Nicolai Stange <nstange@suse.de>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>

diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c
index 04522dc..12f80c5 100644 (file)
--- a/sound/core/seq/seq_clientmgr.c
+++ b/sound/core/seq/seq_clientmgr.c
@@ -907,7 +907,8 @@ int snd_seq_dispatch_event(struct snd_seq_event_cell *cell, int atomic, int hop)
 static int snd_seq_client_enqueue_event(struct snd_seq_client *client,
                                        struct snd_seq_event *event,
                                        struct file *file, int blocking,
 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;
 {
        struct snd_seq_event_cell *cell;
        int err;


@@ -945,7 +946,8 @@ static int snd_seq_client_enqueue_event(struct snd_seq_client *client,
                return -ENXIO; /* queue is not allocated */
 
        /* allocate an event cell */
                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;
 
        if (err < 0)
                return err;
 


@@ -1014,12 +1016,11 @@ static ssize_t snd_seq_write(struct file *file, const char __user *buf,
                return -ENXIO;
 
        /* allocate the pool now if the pool is not allocated yet */ 
                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)) {
        if (client->pool->size > 0 && !snd_seq_write_pool_allocated(client)) {

-               mutex_lock(&client->ioctl_mutex);

                err = snd_seq_pool_init(client->pool);
                err = snd_seq_pool_init(client->pool);

-               mutex_unlock(&client->ioctl_mutex);

                if (err < 0)
                if (err < 0)

-                       return -ENOMEM;
+                       goto out;

        }
 
        /* only process whole events */
        }
 
        /* only process whole events */


@@ -1070,7 +1071,7 @@ static ssize_t snd_seq_write(struct file *file, const char __user *buf,
                /* ok, enqueue it */
                err = snd_seq_client_enqueue_event(client, &event, file,
                                                   !(file->f_flags & O_NONBLOCK),
                /* 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;
 
                if (err < 0)
                        break;
 


@@ -1081,6 +1082,8 @@ static ssize_t snd_seq_write(struct file *file, const char __user *buf,
                written += len;
        }
 
                written += len;
        }
 

+ out:
+       mutex_unlock(&client->ioctl_mutex);

        return written ? written : err;
 }
 
        return written ? written : err;
 }
 


@@ -2343,7 +2346,8 @@ static int kernel_client_enqueue(int client, struct snd_seq_event *ev,
        if (! cptr->accept_output)
                result = -EPERM;
        else /* send it */
        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;
 
        snd_seq_client_unlock(cptr);
        return result;

diff --git a/sound/core/seq/seq_fifo.c b/sound/core/seq/seq_fifo.c
index e4389f1..9d429bc 100644 (file)
--- a/sound/core/seq/seq_fifo.c
+++ b/sound/core/seq/seq_fifo.c
@@ -125,7 +125,7 @@ int snd_seq_fifo_event_in(struct snd_seq_fifo *f,
                return -EINVAL;
 
        snd_use_lock_use(&f->use_lock);
                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);
        if (err < 0) {
                if ((err == -ENOMEM) || (err == -EAGAIN))
                        atomic_inc(&f->overflow);

diff --git a/sound/core/seq/seq_memory.c b/sound/core/seq/seq_memory.c
index 6fc211d..ce5d003 100644 (file)
--- 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_event_cell * cell)
  */
 static int snd_seq_cell_alloc(struct snd_seq_pool *pool,
                              struct snd_seq_event_cell **cellp,
  */
 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;
 {
        struct snd_seq_event_cell *cell;
        unsigned long flags;


@@ -245,7 +246,11 @@ static int snd_seq_cell_alloc(struct snd_seq_pool *pool,
                set_current_state(TASK_INTERRUPTIBLE);
                add_wait_queue(&pool->output_sleep, &wait);
                spin_unlock_irq(&pool->lock);
                set_current_state(TASK_INTERRUPTIBLE);
                add_wait_queue(&pool->output_sleep, &wait);
                spin_unlock_irq(&pool->lock);

+               if (mutexp)
+                       mutex_unlock(mutexp);

                schedule();
                schedule();

+               if (mutexp)
+                       mutex_lock(mutexp);

                spin_lock_irq(&pool->lock);
                remove_wait_queue(&pool->output_sleep, &wait);
                /* interrupted? */
                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,
  */
 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;
 {
        int ncells, err;
        unsigned int extlen;


@@ -305,7 +310,7 @@ int snd_seq_event_dup(struct snd_seq_pool *pool, struct snd_seq_event *event,
        if (ncells >= pool->total_elements)
                return -ENOMEM;
 
        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;
 
        if (err < 0)
                return err;
 


@@ -331,7 +336,8 @@ int snd_seq_event_dup(struct snd_seq_pool *pool, struct snd_seq_event *event,
                        int size = sizeof(struct snd_seq_event);
                        if (len < size)
                                size = len;
                        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)
                        if (err < 0)
                                goto __error;
                        if (cell->event.data.ext.ptr == NULL)

diff --git a/sound/core/seq/seq_memory.h b/sound/core/seq/seq_memory.h
index 32f959c..3abe306 100644 (file)
--- 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,
 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)
 
 /* return number of unused (free) cells */
 static inline int snd_seq_unused_cells(struct snd_seq_pool *pool)