ANDROID: sound: rawmidi: Hold lock around realloc

The SNDRV_RAWMIDI_STREAM_{OUTPUT,INPUT} ioctls may reallocate
runtime->buffer while other kernel threads are accessing it.  If the
underlying krealloc() call frees the original buffer, then this can turn
into a use-after-free.

Most of these accesses happen while the thread is holding runtime->lock,
and can be fixed by just holding the same lock while replacing
runtime->buffer, however we can't hold this spinlock while
snd_rawmidi_kernel_{read1,write1} are copying to/from userspace.  We
need to add and acquire a new mutex to prevent this from happening
concurrently with reallocation.  We hold this mutex during the entire
reallocation process, to also prevent multiple concurrent reallocations
leading to a double-free.

Signed-off-by: Daniel Rosenberg <drosen@google.com>
bug: 64315347
Change-Id: I05764d4f1a38f373eb7c0ac1c98607ee5ff0eded
[dcagle@codeaurora.org: Resolve trivial merge conflict]
Git-repo: https://android.googlesource.com/kernel/msm
Git-commit: d7193540482d11ff0ad3a07fc18717811641c6eb
Signed-off-by: Dennis Cagle <dcagle@codeaurora.org>

diff --git a/include/sound/rawmidi.h b/include/sound/rawmidi.h
index 3b91ad5..2dd096e 100644 (file)
--- a/include/sound/rawmidi.h
+++ b/include/sound/rawmidi.h
@@ -78,6 +78,7 @@ struct snd_rawmidi_runtime {
        size_t xruns;           /* over/underruns counter */
        /* misc */
        spinlock_t lock;
+       struct mutex realloc_mutex;
        wait_queue_head_t sleep;
        /* event handler (new bytes, input only) */
        void (*event)(struct snd_rawmidi_substream *substream);

diff --git a/sound/core/rawmidi.c b/sound/core/rawmidi.c
index 16f8124..5143801 100644 (file)
--- a/sound/core/rawmidi.c
+++ b/sound/core/rawmidi.c
@@ -115,6 +115,7 @@ static int snd_rawmidi_runtime_create(struct snd_rawmidi_substream *substream)
                return -ENOMEM;
        runtime->substream = substream;
        spin_lock_init(&runtime->lock);
+       mutex_init(&runtime->realloc_mutex);
        init_waitqueue_head(&runtime->sleep);
        INIT_WORK(&runtime->event_work, snd_rawmidi_input_event_work);
        runtime->event = NULL;
@@ -636,8 +637,10 @@ int snd_rawmidi_output_params(struct snd_rawmidi_substream *substream,
                              struct snd_rawmidi_params * params)
 {
        char *newbuf;
+       char *oldbuf;
        struct snd_rawmidi_runtime *runtime = substream->runtime;
-       
+       unsigned long flags;
+
        if (substream->append && substream->use_count > 1)
                return -EBUSY;
        snd_rawmidi_drain_output(substream);
@@ -648,13 +651,22 @@ int snd_rawmidi_output_params(struct snd_rawmidi_substream *substream,
                return -EINVAL;
        }
        if (params->buffer_size != runtime->buffer_size) {
-               newbuf = krealloc(runtime->buffer, params->buffer_size,
+               mutex_lock(&runtime->realloc_mutex);
+               newbuf = __krealloc(runtime->buffer, params->buffer_size,
                                  GFP_KERNEL);
-               if (!newbuf)
+               if (!newbuf) {
+                       mutex_unlock(&runtime->realloc_mutex);
                        return -ENOMEM;
+               }
+               spin_lock_irqsave(&runtime->lock, flags);
+               oldbuf = runtime->buffer;
                runtime->buffer = newbuf;
                runtime->buffer_size = params->buffer_size;
                runtime->avail = runtime->buffer_size;
+               spin_unlock_irqrestore(&runtime->lock, flags);
+               if (oldbuf != newbuf)
+                       kfree(oldbuf);
+               mutex_unlock(&runtime->realloc_mutex);
        }
        runtime->avail_min = params->avail_min;
        substream->active_sensing = !params->no_active_sensing;
@@ -666,7 +678,9 @@ int snd_rawmidi_input_params(struct snd_rawmidi_substream *substream,
                             struct snd_rawmidi_params * params)
 {
        char *newbuf;
+       char *oldbuf;
        struct snd_rawmidi_runtime *runtime = substream->runtime;
+       unsigned long flags;
 
        snd_rawmidi_drain_input(substream);
        if (params->buffer_size < 32 || params->buffer_size > 1024L * 1024L) {
@@ -676,12 +690,21 @@ int snd_rawmidi_input_params(struct snd_rawmidi_substream *substream,
                return -EINVAL;
        }
        if (params->buffer_size != runtime->buffer_size) {
-               newbuf = krealloc(runtime->buffer, params->buffer_size,
+               mutex_lock(&runtime->realloc_mutex);
+               newbuf = __krealloc(runtime->buffer, params->buffer_size,
                                  GFP_KERNEL);
-               if (!newbuf)
+               if (!newbuf) {
+                       mutex_unlock(&runtime->realloc_mutex);
                        return -ENOMEM;
+               }
+               spin_lock_irqsave(&runtime->lock, flags);
+               oldbuf = runtime->buffer;
                runtime->buffer = newbuf;
                runtime->buffer_size = params->buffer_size;
+               spin_unlock_irqrestore(&runtime->lock, flags);
+               if (oldbuf != newbuf)
+                       kfree(oldbuf);
+               mutex_unlock(&runtime->realloc_mutex);
        }
        runtime->avail_min = params->avail_min;
        return 0;
@@ -954,6 +977,8 @@ static long snd_rawmidi_kernel_read1(struct snd_rawmidi_substream *substream,
        unsigned long appl_ptr;
 
        spin_lock_irqsave(&runtime->lock, flags);
+       if (userbuf)
+               mutex_lock(&runtime->realloc_mutex);
        while (count > 0 && runtime->avail) {
                count1 = runtime->buffer_size - runtime->appl_ptr;
                if (count1 > count)
@@ -973,6 +998,7 @@ static long snd_rawmidi_kernel_read1(struct snd_rawmidi_substream *substream,
                        spin_unlock_irqrestore(&runtime->lock, flags);
                        if (copy_to_user(userbuf + result,
                                         runtime->buffer + appl_ptr, count1)) {
+                               mutex_unlock(&runtime->realloc_mutex);
                                return result > 0 ? result : -EFAULT;
                        }
                        spin_lock_irqsave(&runtime->lock, flags);
@@ -981,6 +1007,8 @@ static long snd_rawmidi_kernel_read1(struct snd_rawmidi_substream *substream,
                count -= count1;
        }
        spin_unlock_irqrestore(&runtime->lock, flags);
+       if (userbuf)
+               mutex_unlock(&runtime->realloc_mutex);
        return result;
 }
 
@@ -1245,10 +1273,14 @@ static long snd_rawmidi_kernel_write1(struct snd_rawmidi_substream *substream,
                return -EINVAL;
 
        result = 0;
+       if (userbuf)
+               mutex_lock(&runtime->realloc_mutex);
        spin_lock_irqsave(&runtime->lock, flags);
        if (substream->append) {
                if ((long)runtime->avail < count) {
                        spin_unlock_irqrestore(&runtime->lock, flags);
+                       if (userbuf)
+                               mutex_unlock(&runtime->realloc_mutex);
                        return -EAGAIN;
                }
        }
@@ -1284,6 +1316,8 @@ static long snd_rawmidi_kernel_write1(struct snd_rawmidi_substream *substream,
       __end:
        count1 = runtime->avail < runtime->buffer_size;
        spin_unlock_irqrestore(&runtime->lock, flags);
+       if (userbuf)
+               mutex_unlock(&runtime->realloc_mutex);
        if (count1)
                snd_rawmidi_output_trigger(substream, 1);
        return result;