lkml.org 
[lkml]   [2016]   [Jan]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 3.10 17/53] ALSA: timer: Harden slave timer list handling
    Date
    3.10-stable review patch.  If anyone has any objections, please let me know.

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

    From: Takashi Iwai <tiwai@suse.de>

    commit b5a663aa426f4884c71cd8580adae73f33570f0d upstream.

    A slave timer instance might be still accessible in a racy way while
    operating the master instance as it lacks of locking. Since the
    master operation is mostly protected with timer->lock, we should cope
    with it while changing the slave instance, too. Also, some linked
    lists (active_list and ack_list) of slave instances aren't unlinked
    immediately at stopping or closing, and this may lead to unexpected
    accesses.

    This patch tries to address these issues. It adds spin lock of
    timer->lock (either from master or slave, which is equivalent) in a
    few places. For avoiding a deadlock, we ensure that the global
    slave_active_lock is always locked at first before each timer lock.

    Also, ack and active_list of slave instances are properly unlinked at
    snd_timer_stop() and snd_timer_close().

    Last but not least, remove the superfluous call of _snd_timer_stop()
    at removing slave links. This is a noop, and calling it may confuse
    readers wrt locking. Further cleanup will follow in a later patch.

    Actually we've got reports of use-after-free by syzkaller fuzzer, and
    this hopefully fixes these issues.

    Reported-by: Dmitry Vyukov <dvyukov@google.com>
    Signed-off-by: Takashi Iwai <tiwai@suse.de>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

    ---
    sound/core/timer.c | 18 ++++++++++++++----
    1 file changed, 14 insertions(+), 4 deletions(-)

    --- a/sound/core/timer.c
    +++ b/sound/core/timer.c
    @@ -215,11 +215,13 @@ static void snd_timer_check_master(struc
    slave->slave_id == master->slave_id) {
    list_move_tail(&slave->open_list, &master->slave_list_head);
    spin_lock_irq(&slave_active_lock);
    + spin_lock(&master->timer->lock);
    slave->master = master;
    slave->timer = master->timer;
    if (slave->flags & SNDRV_TIMER_IFLG_RUNNING)
    list_add_tail(&slave->active_list,
    &master->slave_active_head);
    + spin_unlock(&master->timer->lock);
    spin_unlock_irq(&slave_active_lock);
    }
    }
    @@ -345,15 +347,18 @@ int snd_timer_close(struct snd_timer_ins
    timer->hw.close)
    timer->hw.close(timer);
    /* remove slave links */
    + spin_lock_irq(&slave_active_lock);
    + spin_lock(&timer->lock);
    list_for_each_entry_safe(slave, tmp, &timeri->slave_list_head,
    open_list) {
    - spin_lock_irq(&slave_active_lock);
    - _snd_timer_stop(slave, 1, SNDRV_TIMER_EVENT_RESOLUTION);
    list_move_tail(&slave->open_list, &snd_timer_slave_list);
    slave->master = NULL;
    slave->timer = NULL;
    - spin_unlock_irq(&slave_active_lock);
    + list_del_init(&slave->ack_list);
    + list_del_init(&slave->active_list);
    }
    + spin_unlock(&timer->lock);
    + spin_unlock_irq(&slave_active_lock);
    mutex_unlock(&register_mutex);
    }
    out:
    @@ -440,9 +445,12 @@ static int snd_timer_start_slave(struct

    spin_lock_irqsave(&slave_active_lock, flags);
    timeri->flags |= SNDRV_TIMER_IFLG_RUNNING;
    - if (timeri->master)
    + if (timeri->master && timeri->timer) {
    + spin_lock(&timeri->timer->lock);
    list_add_tail(&timeri->active_list,
    &timeri->master->slave_active_head);
    + spin_unlock(&timeri->timer->lock);
    + }
    spin_unlock_irqrestore(&slave_active_lock, flags);
    return 1; /* delayed start */
    }
    @@ -488,6 +496,8 @@ static int _snd_timer_stop(struct snd_ti
    if (!keep_flag) {
    spin_lock_irqsave(&slave_active_lock, flags);
    timeri->flags &= ~SNDRV_TIMER_IFLG_RUNNING;
    + list_del_init(&timeri->ack_list);
    + list_del_init(&timeri->active_list);
    spin_unlock_irqrestore(&slave_active_lock, flags);
    }
    goto __end;

    \
     
     \ /
      Last update: 2016-01-27 21:21    [W:2.994 / U:0.076 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site