Fix racy pool initializations & Don't allow resizing pool in use & More protection for concurrent write and ioctl races in kernel-sound-ALSA-seq CVE-2018-7566

Tracked by Jolla

asked 2018-06-25 11:09:22 +0200

this post is marked as community wiki

This post is a wiki. Anyone with karma >75 is welcome to improve it.

updated 2018-06-25 11:09:22 +0200

lpr gravatar image

The Linux kernel 4.15 has a Buffer Overflow via an SNDRV_SEQ_IOCTL_SET_CLIENT_POOL ioctl write operation to /dev/snd/seq by a local user.

Patches are available for 3.2 kernel: |1| |2| |3|

Files affected: kernel-adaptation-sbj-3.4.108.20171107.1/sound/core/seq/seq_clientmgr.c
/sound/core/seq/seq_fifo.c
/sound/core/seq/seq_memory.c
/sound/core/seq/seq_memory.h

so the patches should look like:

--- a/sound/core/seq/seq_clientmgr.c
+++ b/sound/core/seq/seq_clientmgr.c
@@ -903,7 +903,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,
-                   int atomic, int hop)
+                   int atomic, int hop,
+                   struct mutex *mutexp)
 {
struct snd_seq_event_cell *cell;
int err;
@@ -943,7 +944,8 @@ static int snd_seq_client_enqueue_event(struct snd_seq_client *client,
    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;
@@ -996,7 +998,7 @@ static ssize_t snd_seq_write(struct file *file, const char __user *buf,
 {
struct snd_seq_client *client = file->private_data;
int written = 0, len;
-   int err = -EINVAL;
+   int err;
struct snd_seq_event event;

if (!(snd_seq_file_flags(file) & SNDRV_SEQ_LFLG_OUTPUT))

@@ -1012,11 +1014,15 @@ static ssize_t snd_seq_write(struct file *file, const char __user *buf,

/* allocate the pool now if the pool is not allocated yet */ 
if (client->pool->size > 0 && !snd_seq_write_pool_allocated(client)) {
-       if (snd_seq_pool_init(client->pool) < 0)
+       mutex_lock(&client->ioctl_mutex);
+       err = snd_seq_pool_init(client->pool);
+       mutex_unlock(&client->ioctl_mutex);
+       if (err < 0)
        return -ENOMEM;
}

/* only process whole events */
+   err = -EINVAL;
while (count >= sizeof(struct snd_seq_event)) {
    /* Read in the event header from the user */
    len = sizeof(event);


@@ -1012,12 xxxx @@ 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 */ 
+   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 */

@@ -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),
-                          0, 0);
+                          0, 0, &client->ioctl_mutex);
    if (err < 0)
        break;

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

+ out:
+   mutex_unlock(&client->ioctl_mutex);
return written ? written : err;
 }


@@ -1905,6 +1911,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)) {
+           /* is the pool in use? */
+           if (atomic_read(&client->pool->counter))
+               return -EBUSY;
        /* remove all existing cells */
        snd_seq_pool_mark_closing(client->pool);
        snd_seq_queue_client_leave_cells(client->number);

@@ -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 */
-       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;

diff --git a/sound/core/seq/seq_fifo.c b/sound/core/seq/seq_fifo.c
index e4389f1..9d429bc 100644
--- 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);
-   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);
diff --git a/sound/core/seq/seq_memory.c b/sound/core/seq/seq_memory.c
index 6fc211d..ce5d003 100644
--- 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,
-                 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_seq_pool *pool,
        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_pool *pool, struct snd_seq_event *event,
    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_pool *pool, struct snd_seq_event *event,
            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)
diff --git a/sound/core/seq/seq_memory.h b/sound/core/seq/seq_memory.h
index 32f959c..3abe306 100644
--- 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)
edit retag flag offensive close delete