diff options
author | Daniel Rosenberg <drosen@google.com> | 2017-10-31 16:55:26 -0700 |
---|---|---|
committer | Gerrit - the friendly Code Review server <code-review@localhost> | 2018-05-02 05:13:18 -0700 |
commit | 742017e8de6a8d221470ced85a2a481e89b9719d (patch) | |
tree | ff9d9f3964e92748f74f72f47927a04508279d74 /sound/core/rawmidi.c | |
parent | d1507b1236caee1b7efdbffb1f2d16ab387164f5 (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
[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>
Diffstat (limited to 'sound/core/rawmidi.c')
-rw-r--r-- | sound/core/rawmidi.c | 44 |
1 files changed, 39 insertions, 5 deletions
diff --git a/sound/core/rawmidi.c b/sound/core/rawmidi.c index 16f8124b1150..514380104944 100644 --- 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; |