aboutsummaryrefslogtreecommitdiff
path: root/sound
diff options
context:
space:
mode:
authorDaniel Rosenberg <drosen@google.com>2017-10-31 16:55:26 -0700
committerMoyster <oysterized@gmail.com>2018-05-16 13:59:09 +0200
commitd6a5b2bb21682c4b2c634af20ba3d1210ac834de (patch)
tree30c73e5af798560f13293558888711e4cc758eeb /sound
parentad781944055a2b79c3cfb6c5231ab2d5fd0c9a6b (diff)
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
Diffstat (limited to 'sound')
-rw-r--r--sound/core/rawmidi.c48
1 files changed, 41 insertions, 7 deletions
diff --git a/sound/core/rawmidi.c b/sound/core/rawmidi.c
index 3e9761685..930891887 100644
--- a/sound/core/rawmidi.c
+++ b/sound/core/rawmidi.c
@@ -108,6 +108,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;
@@ -622,8 +623,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);
@@ -634,13 +637,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;
@@ -651,7 +663,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) {
@@ -661,12 +675,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;
@@ -936,11 +959,13 @@ static long snd_rawmidi_kernel_read1(struct snd_rawmidi_substream *substream,
struct snd_rawmidi_runtime *runtime = substream->runtime;
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)
count1 = count;
+ spin_lock_irqsave(&runtime->lock, flags);
if (count1 > (int)runtime->avail)
count1 = runtime->avail;
@@ -956,14 +981,17 @@ 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);
}
+ spin_unlock_irqrestore(&runtime->lock, flags);
result += count1;
count -= count1;
}
- spin_unlock_irqrestore(&runtime->lock, flags);
+ if (userbuf)
+ mutex_unlock(&runtime->realloc_mutex);
return result;
}
@@ -1174,10 +1202,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;
}
}
@@ -1213,6 +1245,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;