lkml.org 
[lkml]   [2020]   [May]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 4.4 73/86] ALSA: rawmidi: Fix racy buffer resize under concurrent accesses
    Date
    From: Takashi Iwai <tiwai@suse.de>

    commit c1f6e3c818dd734c30f6a7eeebf232ba2cf3181d 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: 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>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

    ---
    include/sound/rawmidi.h | 1 +
    sound/core/rawmidi.c | 31 +++++++++++++++++++++++++++----
    2 files changed, 28 insertions(+), 4 deletions(-)

    --- a/include/sound/rawmidi.h
    +++ b/include/sound/rawmidi.h
    @@ -76,6 +76,7 @@ struct snd_rawmidi_runtime {
    size_t avail_min; /* min avail for wakeup */
    size_t avail; /* max used buffer for wakeup */
    size_t xruns; /* over/underruns counter */
    + int buffer_ref; /* buffer reference count */
    /* misc */
    spinlock_t lock;
    wait_queue_head_t sleep;
    --- a/sound/core/rawmidi.c
    +++ b/sound/core/rawmidi.c
    @@ -108,6 +108,17 @@ static void snd_rawmidi_input_event_work
    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)
    {
    struct snd_rawmidi_runtime *runtime;
    @@ -654,6 +665,11 @@ int snd_rawmidi_output_params(struct snd
    if (!newbuf)
    return -ENOMEM;
    spin_lock_irq(&runtime->lock);
    + if (runtime->buffer_ref) {
    + spin_unlock_irq(&runtime->lock);
    + kfree(newbuf);
    + return -EBUSY;
    + }
    oldbuf = runtime->buffer;
    runtime->buffer = newbuf;
    runtime->buffer_size = params->buffer_size;
    @@ -962,8 +978,10 @@ static long snd_rawmidi_kernel_read1(str
    long result = 0, count1;
    struct snd_rawmidi_runtime *runtime = substream->runtime;
    unsigned long appl_ptr;
    + int err = 0;

    spin_lock_irqsave(&runtime->lock, flags);
    + snd_rawmidi_buffer_ref(runtime);
    while (count > 0 && runtime->avail) {
    count1 = runtime->buffer_size - runtime->appl_ptr;
    if (count1 > count)
    @@ -982,16 +1000,19 @@ static long snd_rawmidi_kernel_read1(str
    if (userbuf) {
    spin_unlock_irqrestore(&runtime->lock, flags);
    if (copy_to_user(userbuf + result,
    - runtime->buffer + appl_ptr, count1)) {
    - return result > 0 ? result : -EFAULT;
    - }
    + runtime->buffer + appl_ptr, count1))
    + err = -EFAULT;
    spin_lock_irqsave(&runtime->lock, flags);
    + if (err)
    + goto out;
    }
    result += count1;
    count -= count1;
    }
    + out:
    + snd_rawmidi_buffer_unref(runtime);
    spin_unlock_irqrestore(&runtime->lock, flags);
    - return result;
    + return result > 0 ? result : err;
    }

    long snd_rawmidi_kernel_read(struct snd_rawmidi_substream *substream,
    @@ -1262,6 +1283,7 @@ static long snd_rawmidi_kernel_write1(st
    return -EAGAIN;
    }
    }
    + snd_rawmidi_buffer_ref(runtime);
    while (count > 0 && runtime->avail > 0) {
    count1 = runtime->buffer_size - runtime->appl_ptr;
    if (count1 > count)
    @@ -1293,6 +1315,7 @@ static long snd_rawmidi_kernel_write1(st
    }
    __end:
    count1 = runtime->avail < runtime->buffer_size;
    + snd_rawmidi_buffer_unref(runtime);
    spin_unlock_irqrestore(&runtime->lock, flags);
    if (count1)
    snd_rawmidi_output_trigger(substream, 1);

    \
     
     \ /
      Last update: 2020-05-18 20:31    [W:4.075 / U:0.220 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site