linux/kernel/time
Balasubramani Vivekanandan b9023b91dd tick: broadcast-hrtimer: Fix a race in bc_set_next
When a cpu requests broadcasting, before starting the tick broadcast
hrtimer, bc_set_next() checks if the timer callback (bc_handler) is active
using hrtimer_try_to_cancel(). But hrtimer_try_to_cancel() does not provide
the required synchronization when the callback is active on other core.

The callback could have already executed tick_handle_oneshot_broadcast()
and could have also returned. But still there is a small time window where
the hrtimer_try_to_cancel() returns -1. In that case bc_set_next() returns
without doing anything, but the next_event of the tick broadcast clock
device is already set to a timeout value.

In the race condition diagram below, CPU #1 is running the timer callback
and CPU #2 is entering idle state and so calls bc_set_next().

In the worst case, the next_event will contain an expiry time, but the
hrtimer will not be started which happens when the racing callback returns
HRTIMER_NORESTART. The hrtimer might never recover if all further requests
from the CPUs to subscribe to tick broadcast have timeout greater than the
next_event of tick broadcast clock device. This leads to cascading of
failures and finally noticed as rcu stall warnings

Here is a depiction of the race condition

CPU #1 (Running timer callback)                   CPU #2 (Enter idle
                                                  and subscribe to
                                                  tick broadcast)
---------------------                             ---------------------

__run_hrtimer()                                   tick_broadcast_enter()

  bc_handler()                                      __tick_broadcast_oneshot_control()

    tick_handle_oneshot_broadcast()

      raw_spin_lock(&tick_broadcast_lock);

      dev->next_event = KTIME_MAX;                  //wait for tick_broadcast_lock
      //next_event for tick broadcast clock
      set to KTIME_MAX since no other cores
      subscribed to tick broadcasting

      raw_spin_unlock(&tick_broadcast_lock);

    if (dev->next_event == KTIME_MAX)
      return HRTIMER_NORESTART
    // callback function exits without
       restarting the hrtimer                      //tick_broadcast_lock acquired
                                                   raw_spin_lock(&tick_broadcast_lock);

                                                   tick_broadcast_set_event()

                                                     clockevents_program_event()

                                                       dev->next_event = expires;

                                                       bc_set_next()

                                                         hrtimer_try_to_cancel()
                                                         //returns -1 since the timer
                                                         callback is active. Exits without
                                                         restarting the timer
  cpu_base->running = NULL;

The comment that hrtimer cannot be armed from within the callback is
wrong. It is fine to start the hrtimer from within the callback. Also it is
safe to start the hrtimer from the enter/exit idle code while the broadcast
handler is active. The enter/exit idle code and the broadcast handler are
synchronized using tick_broadcast_lock. So there is no need for the
existing try to cancel logic. All this can be removed which will eliminate
the race condition as well.

Fixes: 5d1638acb9 ("tick: Introduce hrtimer based broadcast")
Originally-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Balasubramani Vivekanandan <balasubramani_vivekanandan@mentor.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: stable@vger.kernel.org
Link: https://lkml.kernel.org/r/20190926135101.12102-2-balasubramani_vivekanandan@mentor.com
2019-09-27 14:45:55 +02:00
..
alarmtimer.c Power management updates for 5.4-rc1 2019-09-17 19:15:14 -07:00
clockevents.c tick: Remove outgoing CPU from broadcast masks 2019-03-23 18:26:43 +01:00
clocksource.c clocksource: Move inline keyword to the beginning of function declarations 2019-06-14 17:04:03 +02:00
hrtimer.c hrtimer: Add a missing bracket and hide `migration_base' on !SMP 2019-09-05 10:39:06 +02:00
itimer.c posix-cpu-timers: Switch thread group sampling to array 2019-08-28 11:50:39 +02:00
jiffies.c Merge branch 'timers-core-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip 2019-05-06 14:50:46 -07:00
Kconfig treewide: Add SPDX license identifier - Makefile/Kconfig 2019-05-21 10:50:46 +02:00
Makefile timekeeping: Provide a generic update_vsyscall() implementation 2019-06-22 21:21:06 +02:00
ntp_internal.h ntp: Audit NTP parameters adjustment 2019-04-15 18:14:01 -04:00
ntp.c ntp: Limit TAI-UTC offset 2019-06-22 11:28:53 +02:00
posix-clock.c timex: use __kernel_timex internally 2019-02-07 00:13:27 +01:00
posix-cpu-timers.c posix-cpu-timers: Fix permission check regression 2019-09-10 12:13:07 +01:00
posix-stubs.c y2038: syscalls: rename y2038 compat syscalls 2019-02-07 00:13:27 +01:00
posix-timers.c hrtimer: Improve comments on handling priority inversion against softirq kthread 2019-08-20 22:05:46 +02:00
posix-timers.h posix-timers: Use a callback for cancel synchronization on PREEMPT_RT 2019-08-20 22:05:46 +02:00
sched_clock.c Printk changes for 5.2 2019-05-07 09:18:12 -07:00
test_udelay.c time/debug: Remove license boilerplate 2018-11-23 11:51:21 +01:00
tick-broadcast-hrtimer.c tick: broadcast-hrtimer: Fix a race in bc_set_next 2019-09-27 14:45:55 +02:00
tick-broadcast.c tick: Fix typos in comments 2019-04-19 19:17:04 +02:00
tick-common.c Merge branch 'timers-core-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip 2019-05-06 14:50:46 -07:00
tick-internal.h tick: Remove outgoing CPU from broadcast masks 2019-03-23 18:26:43 +01:00
tick-oneshot.c hrtimers/tick/clockevents: Remove sloppy license references 2018-11-23 11:51:21 +01:00
tick-sched.c tick: Mark sched_timer to expire in hard interrupt context 2019-08-28 13:01:26 +02:00
tick-sched.h tick/sched: Update tick_sched struct documentation 2019-03-24 20:29:32 +01:00
time.c time: Validate user input in compat_settimeofday() 2019-07-07 12:05:40 +02:00
timeconst.bc time: Add SPDX license identifiers 2018-11-23 11:51:20 +01:00
timeconv.c time: Add SPDX license identifiers 2018-11-23 11:51:20 +01:00
timecounter.c time: Remove license boilerplate 2018-11-23 11:51:21 +01:00
timekeeping_debug.c timekeeping/debug: No need to check return value of debugfs_create functions 2019-01-29 20:08:41 +01:00
timekeeping_internal.h timekeeping/ntp: Constify some function arguments 2018-07-19 17:08:05 -07:00
timekeeping.c timekeeping/vsyscall: Prevent math overflow in BOOTTIME update 2019-08-23 02:12:11 +02:00
timekeeping.h timers/sched_clock: Prevent generic sched_clock wrap caused by tick_freeze() 2019-04-18 14:34:53 +02:00
timer_list.c timer_list: Guard procfs specific code 2019-06-23 00:08:52 +02:00
timer.c timer: Read jiffies once when forwarding base clk 2019-09-19 17:50:11 +02:00
vsyscall.c timekeeping/vsyscall: Prevent math overflow in BOOTTIME update 2019-08-23 02:12:11 +02:00