It seems like commit: 22b886dd timers: Use proper base migration in add_timer_on() [1]
Has been started to by apply :
$ git tag --contains 22b886dd
Ubuntu-lts-4.4.0-4.19_14.04.1
Ubuntu-lts-4.4.0-4.19_14.04.2
[1] $ git show 22b886dd
commit 22b886dd1018093920c4250dee2a9a3cb7cff7b8
Author: Tejun Heo <email address hidden>
Date: Wed Nov 4 12:15:33 2015 -0500
timers: Use proper base migration in add_timer_on()
Regardless of the previous CPU a timer was on, add_timer_on()
currently simply sets timer->flags to the new CPU. As the caller must
be seeing the timer as idle, this is locally fine, but the timer
leaving the old base while unlocked can lead to race conditions as
follows.
Let's say timer was on cpu 0.
cpu 0 cpu 1
-----------------------------------------------------------------------------
del_timer(timer) succeeds del_timer(timer) lock_timer_base(timer) locks cpu_0_base
add_timer_on(timer, 1) spin_lock(&cpu_1_base->lock) timer->flags set to cpu_1_base
operates on @timer operates on @timer
This triggered with mod_delayed_work_on() which contains
"if (del_timer()) add_timer_on()" sequence eventually leading to the
following oops.
timer_stats_timer_set_start_info(timer); BUG_ON(timer_pending(timer) || !timer->function);
- spin_lock_irqsave(&base->lock, flags);
- timer->flags = (timer->flags & ~TIMER_BASEMASK) | cpu;
+
+ /*
+ * If @timer was on a different CPU, it should be migrated with the
+ * old base locked to prevent other operations proceeding with the
+ * wrong base locked. See lock_timer_base().
+ */
+ base = lock_timer_base(timer, &flags);
+ if (base != new_base) {
+ timer->flags |= TIMER_MIGRATING;
+
+ spin_unlock(&base->lock);
+ base = new_base;
+ spin_lock(&base->lock);
+ WRITE_ONCE(timer->flags,
+ (timer->flags & ~TIMER_BASEMASK) | cpu);
+ }
+ debug_activate(timer, timer->expires); internal_add_timer(base, timer); spin_unlock_irqrestore(&base->lock, flags);
It seems like commit: 22b886dd timers: Use proper base migration in add_timer_on() [1]
Has been started to by apply :
$ git tag --contains 22b886dd lts-4.4. 0-4.19_ 14.04.1 lts-4.4. 0-4.19_ 14.04.2
Ubuntu-
Ubuntu-
[1] $ git show 22b886dd 920c4250dee2a9a 3cb7cff7b8
commit 22b886dd1018093
Author: Tejun Heo <email address hidden>
Date: Wed Nov 4 12:15:33 2015 -0500
timers: Use proper base migration in add_timer_on()
Regardless of the previous CPU a timer was on, add_timer_on()
currently simply sets timer->flags to the new CPU. As the caller must
be seeing the timer as idle, this is locally fine, but the timer
leaving the old base while unlocked can lead to race conditions as
follows.
Let's say timer was on cpu 0.
cpu 0 cpu 1 ------- ------- ------- ------- ------- ------- ------- ------- ------- ------- ------ timer(timer) succeeds
del_ timer(timer)
lock_timer_ base(timer) locks cpu_0_base timer_on( timer, 1)
spin_lock( &cpu_1_ base->lock)
timer- >flags set to cpu_1_base
-
del_
add_
operates on @timer operates on @timer
This triggered with mod_delayed_ work_on( ) which contains
"if (del_timer()) add_timer_on()" sequence eventually leading to the
following oops.
BUG: unable to handle kernel NULL pointer dereference at (null) 6e9>] detach_ if_pending+ 0x69/0x1a0 ffffffff810ca6e 9>] [<ffffffff810ca 6e9>] detach_ if_pending+ 0x69/0x1a0
[<ffffffff810cb 0b4>] del_timer+0x44/0x60
[<ffffffff8106e 836>] try_to_ grab_pending+ 0xb6/0x160
[<ffffffff8106e 913>] mod_delayed_ work_on+ 0x33/0x80
[<ffffffffa0000 081>] wqthrash_ workfunc+ 0x61/0x90 [wqthrash]
[<ffffffff8106d ba8>] process_ one_work+ 0x1e8/0x650
[<ffffffff8106e 05e>] worker_ thread+ 0x4e/0x450
[<ffffffff81074 6af>] kthread+0xef/0x110
[<ffffffff81859 80f>] ret_from_ fork+0x3f/ 0x70
IP: [<ffffffff810ca
...
Workqueue: wqthrash wqthrash_workfunc [wqthrash]
task: ffff8800172ca680 ti: ffff8800172d0000 task.ti: ffff8800172d0000
RIP: 0010:[<
...
Call Trace:
Fix it by updating add_timer_on() to perform proper migration as
__mod_timer() does.
Reported- and-tested- by: Jeff Layton <email address hidden>
Signed-off-by: Tejun Heo <email address hidden>
Cc: Chris Worley <email address hidden>
Cc: <email address hidden>
Cc: Michael Skralivetsky <email address hidden>
Cc: Trond Myklebust <email address hidden>
Cc: Shaohua Li <email address hidden>
Cc: Jeff Layton <email address hidden>
Cc: <email address hidden>
Cc: <email address hidden>
Link: http://<email address hidden>
Link: http://<email address hidden>
Signed-off-by: Thomas Gleixner <email address hidden>
diff --git a/kernel/ time/timer. c b/kernel/ time/timer. c time/timer. c time/timer. c SYMBOL( add_timer) ; ptr(&tvec_ bases, cpu); ptr(&tvec_ bases, cpu);
index 74591ba..bbc5d11 100644
--- a/kernel/
+++ b/kernel/
@@ -977,13 +977,29 @@ EXPORT_
*/
void add_timer_on(struct timer_list *timer, int cpu)
{
- struct tvec_base *base = per_cpu_
+ struct tvec_base *new_base = per_cpu_
+ struct tvec_base *base;
unsigned long flags;
- spin_lock_
- timer->flags = (timer->flags & ~TIMER_BASEMASK) | cpu;
+
+ /*
+ * If @timer was on a different CPU, it should be migrated with the
+ * old base locked to prevent other operations proceeding with the
+ * wrong base locked. See lock_timer_base().
+ */
+ base = lock_timer_
+ if (base != new_base) {
+ timer->flags |= TIMER_MIGRATING;
+
+ spin_unlock(
+ base = new_base;
+ spin_lock(
+ WRITE_ONCE(
+ (timer->flags & ~TIMER_BASEMASK) | cpu);
+ }
+