Skip to contentSkip to conversation

Patch "timers: Prevent base clock corruption when forwarding" has been added to the 4.8-stable tree

PATCH [email protected] 1 participant 1 message Nov 7, 2016

Subsystems (2)
lore
Share & tools
lore

[email protected]

author maintainer Applied
#0 lore raw

This is a note to let you know that I've just added the patch titled

timers: Prevent base clock corruption when forwarding

to the 4.8-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
timers-prevent-base-clock-corruption-when-forwarding.patch
and it can be found in the queue-4.8 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree, please let <[email protected]> know about it.

Quoted 1 line

From 6bad6bccf2d717f652d37e63cf261eaa23466009 Mon Sep 17 00:00:00 2001

From: Thomas Gleixner <[email protected]>
Date: Sat, 22 Oct 2016 11:07:37 +0000
Subject: timers: Prevent base clock corruption when forwarding

From: Thomas Gleixner <[email protected]>

commit 6bad6bccf2d717f652d37e63cf261eaa23466009 upstream.

When a timer is enqueued we try to forward the timer base clock. This mechanism has two issues:

1) Forwarding a remote base unlocked

The forwarding function is called from get_target_base() with the current timer base lock held. But if the new target base is a different base than the current base (can happen with NOHZ, sigh!) then the forwarding is done on an unlocked base. This can lead to corruption of base->clk.

Solution is simple: Invoke the forwarding after the target base is locked.

2) Possible corruption due to jiffies advancing

This is similar to the issue in get_net_timer_interrupt() which was fixed in the previous patch. jiffies can advance between check and assignement and therefore advancing base->clk beyond the next expiry value.

So we need to read jiffies into a local variable once and do the checks and assignment with the local copy.

---
 kernel/time/timer.c |   23 ++++++++++-------------
 1 file changed, 10 insertions(+), 13 deletions(-)
kernel/time/timer.c +10 -13
@@ -878,7 +878,7 @@ static inline struct timer_base *get_tim
 
 #ifdef CONFIG_NO_HZ_COMMON
 static inline struct timer_base *
-__get_target_base(struct timer_base *base, unsigned tflags)
+get_target_base(struct timer_base *base, unsigned tflags)
 {
 #ifdef CONFIG_SMP
 	if ((tflags & TIMER_PINNED) || !base->migration_enabled)
@@ -891,25 +891,27 @@ __get_target_base(struct timer_base *bas
 
 static inline void forward_timer_base(struct timer_base *base)
 {
+	unsigned long jnow = READ_ONCE(jiffies);
+
 	/*
 	 * We only forward the base when it's idle and we have a delta between
 	 * base clock and jiffies.
 	 */
-	if (!base->is_idle || (long) (jiffies - base->clk) < 2)
+	if (!base->is_idle || (long) (jnow - base->clk) < 2)
 		return;
 
 	/*
 	 * If the next expiry value is > jiffies, then we fast forward to
 	 * jiffies otherwise we forward to the next expiry value.
 	 */
-	if (time_after(base->next_expiry, jiffies))
-		base->clk = jiffies;
+	if (time_after(base->next_expiry, jnow))
+		base->clk = jnow;
 	else
 		base->clk = base->next_expiry;
 }
 #else
 static inline struct timer_base *
-__get_target_base(struct timer_base *base, unsigned tflags)
+get_target_base(struct timer_base *base, unsigned tflags)
 {
 	return get_timer_this_cpu_base(tflags);
 }
@@ -917,14 +919,6 @@ __get_target_base(struct timer_base *bas
 static inline void forward_timer_base(struct timer_base *base) { }
 #endif
 
-static inline struct timer_base *
-get_target_base(struct timer_base *base, unsigned tflags)
-{
-	struct timer_base *target = __get_target_base(base, tflags);
-
-	forward_timer_base(target);
-	return target;
-}
 
 /*
  * We are using hashed locking: Holding per_cpu(timer_bases[x]).lock means
@@ -1025,6 +1019,9 @@ __mod_timer(struct timer_list *timer, un
 		}
 	}
 
+	/* Try to forward a stale timer base clock */
+	forward_timer_base(base);
+
 	timer->expires = expires;
 	/*
 	 * If 'idx' was calculated above and the base time did not advance

Patches currently in stable-queue which might be from [email protected] are

queue-4.8/x86-microcode-amd-fix-more-fallout-from-config_randomize_memory-y.patch queue-4.8/timers-lock-base-for-same-bucket-optimization.patch queue-4.8/timers-plug-locking-race-vs.-timer-migration.patch queue-4.8/timers-prevent-base-clock-corruption-when-forwarding.patch queue-4.8/timers-prevent-base-clock-rewind-when-forwarding-clock.patch