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>
Signed-off-by: Aaditya Vohra <vohraa@codeaurora.org>
tirimbino
Daniel Rosenberg 7 years ago committed by Dennis Cagle
parent cb57e6b443
commit bbfa49a952
  1. 1
      include/sound/rawmidi.h
  2. 44
      sound/core/rawmidi.c

@ -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);

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

Loading…
Cancel
Save