lkml.org 
[lkml]   [2020]   [May]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Date
    Subject[PATCH 3.16 91/99] clocksource: Prevent double add_timer_on() for watchdog_timer
    3.16.84-rc1 review patch.  If anyone has any objections, please let me know.

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

    From: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>

    commit febac332a819f0e764aa4da62757ba21d18c182b upstream.

    Kernel crashes inside QEMU/KVM are observed:

    kernel BUG at kernel/time/timer.c:1154!
    BUG_ON(timer_pending(timer) || !timer->function) in add_timer_on().

    At the same time another cpu got:

    general protection fault: 0000 [#1] SMP PTI of poinson pointer 0xdead000000000200 in:

    __hlist_del at include/linux/list.h:681
    (inlined by) detach_timer at kernel/time/timer.c:818
    (inlined by) expire_timers at kernel/time/timer.c:1355
    (inlined by) __run_timers at kernel/time/timer.c:1686
    (inlined by) run_timer_softirq at kernel/time/timer.c:1699

    Unfortunately kernel logs are badly scrambled, stacktraces are lost.

    Printing the timer->function before the BUG_ON() pointed to
    clocksource_watchdog().

    The execution of clocksource_watchdog() can race with a sequence of
    clocksource_stop_watchdog() .. clocksource_start_watchdog():

    expire_timers()
    detach_timer(timer, true);
    timer->entry.pprev = NULL;
    raw_spin_unlock_irq(&base->lock);
    call_timer_fn
    clocksource_watchdog()

    clocksource_watchdog_kthread() or
    clocksource_unbind()

    spin_lock_irqsave(&watchdog_lock, flags);
    clocksource_stop_watchdog();
    del_timer(&watchdog_timer);
    watchdog_running = 0;
    spin_unlock_irqrestore(&watchdog_lock, flags);

    spin_lock_irqsave(&watchdog_lock, flags);
    clocksource_start_watchdog();
    add_timer_on(&watchdog_timer, ...);
    watchdog_running = 1;
    spin_unlock_irqrestore(&watchdog_lock, flags);

    spin_lock(&watchdog_lock);
    add_timer_on(&watchdog_timer, ...);
    BUG_ON(timer_pending(timer) || !timer->function);
    timer_pending() -> true
    BUG()

    I.e. inside clocksource_watchdog() watchdog_timer could be already armed.

    Check timer_pending() before calling add_timer_on(). This is sufficient as
    all operations are synchronized by watchdog_lock.

    Fixes: 75c5158f70c0 ("timekeeping: Update clocksource with stop_machine")
    Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
    Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
    Link: https://lore.kernel.org/r/158048693917.4378.13823603769948933793.stgit@buzz
    Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
    ---
    kernel/time/clocksource.c | 11 +++++++++--
    1 file changed, 9 insertions(+), 2 deletions(-)

    --- a/kernel/time/clocksource.c
    +++ b/kernel/time/clocksource.c
    @@ -343,8 +343,15 @@ static void clocksource_watchdog(unsigne
    next_cpu = cpumask_next(raw_smp_processor_id(), cpu_online_mask);
    if (next_cpu >= nr_cpu_ids)
    next_cpu = cpumask_first(cpu_online_mask);
    - watchdog_timer.expires += WATCHDOG_INTERVAL;
    - add_timer_on(&watchdog_timer, next_cpu);
    +
    + /*
    + * Arm timer if not already pending: could race with concurrent
    + * pair clocksource_stop_watchdog() clocksource_start_watchdog().
    + */
    + if (!timer_pending(&watchdog_timer)) {
    + watchdog_timer.expires += WATCHDOG_INTERVAL;
    + add_timer_on(&watchdog_timer, next_cpu);
    + }
    out:
    spin_unlock(&watchdog_lock);
    }
    \
     
     \ /
      Last update: 2020-05-20 16:27    [W:4.085 / U:0.028 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site