Commit fb764c8f authored by Takashi Iwai's avatar Takashi Iwai Committed by Marcelo Henrique Cerri

ALSA: rawmidi: Fix racy buffer resize under concurrent accesses

BugLink: https://bugs.launchpad.net/bugs/1881356

commit c1f6e3c8 upstream.

The rawmidi core allows user to resize the runtime buffer via ioctl,
and this may lead to UAF when performed during concurrent reads or
writes: the read/write functions unlock the runtime lock temporarily
during copying form/to user-space, and that's the race window.

This patch fixes the hole by introducing a reference counter for the
runtime buffer read/write access and returns -EBUSY error when the
resize is performed concurrently against read/write.

Note that the ref count field is a simple integer instead of
refcount_t here, since the all contexts accessing the buffer is
basically protected with a spinlock, hence we need no expensive atomic
ops.  Also, note that this busy check is needed only against read /
write functions, and not in receive/transmit callbacks; the race can
happen only at the spinlock hole mentioned in the above, while the
whole function is protected for receive / transmit callbacks.
Reported-by: default avatarbutt3rflyh4ck <butterflyhuangxx@gmail.com>
Cc: <stable@vger.kernel.org>
Link: https://lore.kernel.org/r/CAFcO6XMWpUVK_yzzCpp8_XP7+=oUpQvuBeCbMffEDkpe8jWrfg@mail.gmail.com
Link: https://lore.kernel.org/r/s5heerw3r5z.wl-tiwai@suse.deSigned-off-by: default avatarTakashi Iwai <tiwai@suse.de>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: default avatarIan May <ian.may@canonical.com>
Signed-off-by: default avatarKelsey Skunberg <kelsey.skunberg@canonical.com>
parent fb2b1122
...@@ -76,6 +76,7 @@ struct snd_rawmidi_runtime { ...@@ -76,6 +76,7 @@ struct snd_rawmidi_runtime {
size_t avail_min; /* min avail for wakeup */ size_t avail_min; /* min avail for wakeup */
size_t avail; /* max used buffer for wakeup */ size_t avail; /* max used buffer for wakeup */
size_t xruns; /* over/underruns counter */ size_t xruns; /* over/underruns counter */
int buffer_ref; /* buffer reference count */
/* misc */ /* misc */
spinlock_t lock; spinlock_t lock;
wait_queue_head_t sleep; wait_queue_head_t sleep;
......
...@@ -108,6 +108,17 @@ static void snd_rawmidi_input_event_work(struct work_struct *work) ...@@ -108,6 +108,17 @@ static void snd_rawmidi_input_event_work(struct work_struct *work)
runtime->event(runtime->substream); runtime->event(runtime->substream);
} }
/* buffer refcount management: call with runtime->lock held */
static inline void snd_rawmidi_buffer_ref(struct snd_rawmidi_runtime *runtime)
{
runtime->buffer_ref++;
}
static inline void snd_rawmidi_buffer_unref(struct snd_rawmidi_runtime *runtime)
{
runtime->buffer_ref--;
}
static int snd_rawmidi_runtime_create(struct snd_rawmidi_substream *substream) static int snd_rawmidi_runtime_create(struct snd_rawmidi_substream *substream)
{ {
struct snd_rawmidi_runtime *runtime; struct snd_rawmidi_runtime *runtime;
...@@ -654,6 +665,11 @@ int snd_rawmidi_output_params(struct snd_rawmidi_substream *substream, ...@@ -654,6 +665,11 @@ int snd_rawmidi_output_params(struct snd_rawmidi_substream *substream,
if (!newbuf) if (!newbuf)
return -ENOMEM; return -ENOMEM;
spin_lock_irq(&runtime->lock); spin_lock_irq(&runtime->lock);
if (runtime->buffer_ref) {
spin_unlock_irq(&runtime->lock);
kfree(newbuf);
return -EBUSY;
}
oldbuf = runtime->buffer; oldbuf = runtime->buffer;
runtime->buffer = newbuf; runtime->buffer = newbuf;
runtime->buffer_size = params->buffer_size; runtime->buffer_size = params->buffer_size;
...@@ -962,8 +978,10 @@ static long snd_rawmidi_kernel_read1(struct snd_rawmidi_substream *substream, ...@@ -962,8 +978,10 @@ static long snd_rawmidi_kernel_read1(struct snd_rawmidi_substream *substream,
long result = 0, count1; long result = 0, count1;
struct snd_rawmidi_runtime *runtime = substream->runtime; struct snd_rawmidi_runtime *runtime = substream->runtime;
unsigned long appl_ptr; unsigned long appl_ptr;
int err = 0;
spin_lock_irqsave(&runtime->lock, flags); spin_lock_irqsave(&runtime->lock, flags);
snd_rawmidi_buffer_ref(runtime);
while (count > 0 && runtime->avail) { while (count > 0 && runtime->avail) {
count1 = runtime->buffer_size - runtime->appl_ptr; count1 = runtime->buffer_size - runtime->appl_ptr;
if (count1 > count) if (count1 > count)
...@@ -982,16 +1000,19 @@ static long snd_rawmidi_kernel_read1(struct snd_rawmidi_substream *substream, ...@@ -982,16 +1000,19 @@ static long snd_rawmidi_kernel_read1(struct snd_rawmidi_substream *substream,
if (userbuf) { if (userbuf) {
spin_unlock_irqrestore(&runtime->lock, flags); spin_unlock_irqrestore(&runtime->lock, flags);
if (copy_to_user(userbuf + result, if (copy_to_user(userbuf + result,
runtime->buffer + appl_ptr, count1)) { runtime->buffer + appl_ptr, count1))
return result > 0 ? result : -EFAULT; err = -EFAULT;
}
spin_lock_irqsave(&runtime->lock, flags); spin_lock_irqsave(&runtime->lock, flags);
if (err)
goto out;
} }
result += count1; result += count1;
count -= count1; count -= count1;
} }
out:
snd_rawmidi_buffer_unref(runtime);
spin_unlock_irqrestore(&runtime->lock, flags); spin_unlock_irqrestore(&runtime->lock, flags);
return result; return result > 0 ? result : err;
} }
long snd_rawmidi_kernel_read(struct snd_rawmidi_substream *substream, long snd_rawmidi_kernel_read(struct snd_rawmidi_substream *substream,
...@@ -1262,6 +1283,7 @@ static long snd_rawmidi_kernel_write1(struct snd_rawmidi_substream *substream, ...@@ -1262,6 +1283,7 @@ static long snd_rawmidi_kernel_write1(struct snd_rawmidi_substream *substream,
return -EAGAIN; return -EAGAIN;
} }
} }
snd_rawmidi_buffer_ref(runtime);
while (count > 0 && runtime->avail > 0) { while (count > 0 && runtime->avail > 0) {
count1 = runtime->buffer_size - runtime->appl_ptr; count1 = runtime->buffer_size - runtime->appl_ptr;
if (count1 > count) if (count1 > count)
...@@ -1293,6 +1315,7 @@ static long snd_rawmidi_kernel_write1(struct snd_rawmidi_substream *substream, ...@@ -1293,6 +1315,7 @@ static long snd_rawmidi_kernel_write1(struct snd_rawmidi_substream *substream,
} }
__end: __end:
count1 = runtime->avail < runtime->buffer_size; count1 = runtime->avail < runtime->buffer_size;
snd_rawmidi_buffer_unref(runtime);
spin_unlock_irqrestore(&runtime->lock, flags); spin_unlock_irqrestore(&runtime->lock, flags);
if (count1) if (count1)
snd_rawmidi_output_trigger(substream, 1); snd_rawmidi_output_trigger(substream, 1);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment