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: Yogendra Charya Tangalapally <c_yctang@qti.qualcomm.com>
This commit is contained in:
Yogendra Charya Tangalapally 2018-02-20 12:33:16 +05:30 committed by TARKZiM
parent ee0d4872ab
commit 84d4c5a179
2 changed files with 39 additions and 5 deletions

View File

@ -77,6 +77,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);

View File

@ -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,9 @@ 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 +636,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 +662,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 +674,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;
@ -935,6 +957,8 @@ static long snd_rawmidi_kernel_read1(struct snd_rawmidi_substream *substream,
long result = 0, count1;
struct snd_rawmidi_runtime *runtime = substream->runtime;
if (userbuf)
mutex_lock(&runtime->realloc_mutex);
while (count > 0 && runtime->avail) {
count1 = runtime->buffer_size - runtime->appl_ptr;
if (count1 > count)
@ -948,6 +972,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 + runtime->appl_ptr, count1)) {
mutex_unlock(&runtime->realloc_mutex);
return result > 0 ? result : -EFAULT;
}
spin_lock_irqsave(&runtime->lock, flags);
@ -959,6 +984,8 @@ static long snd_rawmidi_kernel_read1(struct snd_rawmidi_substream *substream,
result += count1;
count -= count1;
}
if (userbuf)
mutex_unlock(&runtime->realloc_mutex);
return result;
}
@ -1168,10 +1195,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;
}
}
@ -1203,6 +1234,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;