ALSA: rawmidi: Fix racy buffer resize under concurrent accesses
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: butt3rflyh4ck <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.de Signed-off-by: Takashi Iwai <tiwai@suse.de>
This commit is contained in:
parent
da7a8f1a8f
commit
c1f6e3c818
|
@ -61,6 +61,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;
|
||||||
|
|
|
@ -120,6 +120,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;
|
||||||
|
@ -669,6 +680,11 @@ static int resize_runtime_buffer(struct snd_rawmidi_runtime *runtime,
|
||||||
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);
|
||||||
|
kvfree(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;
|
||||||
|
@ -1019,8 +1035,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)
|
||||||
|
@ -1039,16 +1057,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,
|
||||||
|
@ -1342,6 +1363,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)
|
||||||
|
@ -1373,6 +1395,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);
|
||||||
|
|
Loading…
Reference in New Issue