lkml.org 
[lkml]   [2016]   [Mar]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    Subject[PATCH 2.6.32 41/55] ALSA: seq: Fix yet another races among ALSA timer accesses
    2.6.32-longterm review patch.  If anyone has any objections, please let me know.

    ------------------

    From: Takashi Iwai <tiwai@suse.de>

    commit 2cdc7b636d55cbcf42e1e6c8accd85e62d3e9ae8 upstream.

    ALSA sequencer may open/close and control ALSA timer instance
    dynamically either via sequencer events or direct ioctls. These are
    done mostly asynchronously, and it may call still some timer action
    like snd_timer_start() while another is calling snd_timer_close().
    Since the instance gets removed by snd_timer_close(), it may lead to
    a use-after-free.

    This patch tries to address such a race by protecting each
    snd_timer_*() call via the existing spinlock and also by avoiding the
    access to timer during close call.

    BugLink: http://lkml.kernel.org/r/CACT4Y+Z6RzW5MBr-HUdV-8zwg71WQfKTdPpYGvOeS7v4cyurNQ@mail.gmail.com
    Reported-by: Dmitry Vyukov <dvyukov@google.com>
    Signed-off-by: Takashi Iwai <tiwai@suse.de>
    Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
    Signed-off-by: Willy Tarreau <w@1wt.eu>
    ---
    sound/core/seq/seq_timer.c | 87 +++++++++++++++++++++++++++++++++++-----------
    1 file changed, 67 insertions(+), 20 deletions(-)

    diff --git a/sound/core/seq/seq_timer.c b/sound/core/seq/seq_timer.c
    index c2ec4ef..4a9edcb 100644
    --- a/sound/core/seq/seq_timer.c
    +++ b/sound/core/seq/seq_timer.c
    @@ -93,6 +93,9 @@ void snd_seq_timer_delete(struct snd_seq_timer **tmr)

    void snd_seq_timer_defaults(struct snd_seq_timer * tmr)
    {
    + unsigned long flags;
    +
    + spin_lock_irqsave(&tmr->lock, flags);
    /* setup defaults */
    tmr->ppq = 96; /* 96 PPQ */
    tmr->tempo = 500000; /* 120 BPM */
    @@ -108,21 +111,25 @@ void snd_seq_timer_defaults(struct snd_seq_timer * tmr)
    tmr->preferred_resolution = seq_default_timer_resolution;

    tmr->skew = tmr->skew_base = SKEW_BASE;
    + spin_unlock_irqrestore(&tmr->lock, flags);
    }

    -void snd_seq_timer_reset(struct snd_seq_timer * tmr)
    +static void seq_timer_reset(struct snd_seq_timer *tmr)
    {
    - unsigned long flags;
    -
    - spin_lock_irqsave(&tmr->lock, flags);
    -
    /* reset time & songposition */
    tmr->cur_time.tv_sec = 0;
    tmr->cur_time.tv_nsec = 0;

    tmr->tick.cur_tick = 0;
    tmr->tick.fraction = 0;
    +}
    +
    +void snd_seq_timer_reset(struct snd_seq_timer *tmr)
    +{
    + unsigned long flags;

    + spin_lock_irqsave(&tmr->lock, flags);
    + seq_timer_reset(tmr);
    spin_unlock_irqrestore(&tmr->lock, flags);
    }

    @@ -141,8 +148,11 @@ static void snd_seq_timer_interrupt(struct snd_timer_instance *timeri,
    tmr = q->timer;
    if (tmr == NULL)
    return;
    - if (!tmr->running)
    + spin_lock_irqsave(&tmr->lock, flags);
    + if (!tmr->running) {
    + spin_unlock_irqrestore(&tmr->lock, flags);
    return;
    + }

    resolution *= ticks;
    if (tmr->skew != tmr->skew_base) {
    @@ -151,8 +161,6 @@ static void snd_seq_timer_interrupt(struct snd_timer_instance *timeri,
    (((resolution & 0xffff) * tmr->skew) >> 16);
    }

    - spin_lock_irqsave(&tmr->lock, flags);
    -
    /* update timer */
    snd_seq_inc_time_nsec(&tmr->cur_time, resolution);

    @@ -299,26 +307,30 @@ int snd_seq_timer_open(struct snd_seq_queue *q)
    t->callback = snd_seq_timer_interrupt;
    t->callback_data = q;
    t->flags |= SNDRV_TIMER_IFLG_AUTO;
    + spin_lock_irq(&tmr->lock);
    tmr->timeri = t;
    + spin_unlock_irq(&tmr->lock);
    return 0;
    }

    int snd_seq_timer_close(struct snd_seq_queue *q)
    {
    struct snd_seq_timer *tmr;
    + struct snd_timer_instance *t;

    tmr = q->timer;
    if (snd_BUG_ON(!tmr))
    return -EINVAL;
    - if (tmr->timeri) {
    - snd_timer_stop(tmr->timeri);
    - snd_timer_close(tmr->timeri);
    - tmr->timeri = NULL;
    - }
    + spin_lock_irq(&tmr->lock);
    + t = tmr->timeri;
    + tmr->timeri = NULL;
    + spin_unlock_irq(&tmr->lock);
    + if (t)
    + snd_timer_close(t);
    return 0;
    }

    -int snd_seq_timer_stop(struct snd_seq_timer * tmr)
    +static int seq_timer_stop(struct snd_seq_timer *tmr)
    {
    if (! tmr->timeri)
    return -EINVAL;
    @@ -329,6 +341,17 @@ int snd_seq_timer_stop(struct snd_seq_timer * tmr)
    return 0;
    }

    +int snd_seq_timer_stop(struct snd_seq_timer *tmr)
    +{
    + unsigned long flags;
    + int err;
    +
    + spin_lock_irqsave(&tmr->lock, flags);
    + err = seq_timer_stop(tmr);
    + spin_unlock_irqrestore(&tmr->lock, flags);
    + return err;
    +}
    +
    static int initialize_timer(struct snd_seq_timer *tmr)
    {
    struct snd_timer *t;
    @@ -361,13 +384,13 @@ static int initialize_timer(struct snd_seq_timer *tmr)
    return 0;
    }

    -int snd_seq_timer_start(struct snd_seq_timer * tmr)
    +static int seq_timer_start(struct snd_seq_timer *tmr)
    {
    if (! tmr->timeri)
    return -EINVAL;
    if (tmr->running)
    - snd_seq_timer_stop(tmr);
    - snd_seq_timer_reset(tmr);
    + seq_timer_stop(tmr);
    + seq_timer_reset(tmr);
    if (initialize_timer(tmr) < 0)
    return -EINVAL;
    snd_timer_start(tmr->timeri, tmr->ticks);
    @@ -376,14 +399,25 @@ int snd_seq_timer_start(struct snd_seq_timer * tmr)
    return 0;
    }

    -int snd_seq_timer_continue(struct snd_seq_timer * tmr)
    +int snd_seq_timer_start(struct snd_seq_timer *tmr)
    +{
    + unsigned long flags;
    + int err;
    +
    + spin_lock_irqsave(&tmr->lock, flags);
    + err = seq_timer_start(tmr);
    + spin_unlock_irqrestore(&tmr->lock, flags);
    + return err;
    +}
    +
    +static int seq_timer_continue(struct snd_seq_timer *tmr)
    {
    if (! tmr->timeri)
    return -EINVAL;
    if (tmr->running)
    return -EBUSY;
    if (! tmr->initialized) {
    - snd_seq_timer_reset(tmr);
    + seq_timer_reset(tmr);
    if (initialize_timer(tmr) < 0)
    return -EINVAL;
    }
    @@ -393,11 +427,24 @@ int snd_seq_timer_continue(struct snd_seq_timer * tmr)
    return 0;
    }

    +int snd_seq_timer_continue(struct snd_seq_timer *tmr)
    +{
    + unsigned long flags;
    + int err;
    +
    + spin_lock_irqsave(&tmr->lock, flags);
    + err = seq_timer_continue(tmr);
    + spin_unlock_irqrestore(&tmr->lock, flags);
    + return err;
    +}
    +
    /* return current 'real' time. use timeofday() to get better granularity. */
    snd_seq_real_time_t snd_seq_timer_get_cur_time(struct snd_seq_timer *tmr)
    {
    snd_seq_real_time_t cur_time;
    + unsigned long flags;

    + spin_lock_irqsave(&tmr->lock, flags);
    cur_time = tmr->cur_time;
    if (tmr->running) {
    struct timeval tm;
    @@ -413,7 +460,7 @@ snd_seq_real_time_t snd_seq_timer_get_cur_time(struct snd_seq_timer *tmr)
    }
    snd_seq_sanity_real_time(&cur_time);
    }
    -
    + spin_unlock_irqrestore(&tmr->lock, flags);
    return cur_time;
    }

    --
    1.7.12.2.21.g234cd45.dirty


    \
     
     \ /
      Last update: 2016-03-04 17:41    [W:4.106 / U:0.052 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site