Commit 6bad6bcc authored by Thomas Gleixner's avatar Thomas Gleixner
Browse files

timers: Prevent base clock corruption when forwarding 



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.

Fixes: a683f390("timers: Forward the wheel clock whenever possible")
Reported-by: default avatarAshton Holmes <scoopta@gmail.com>
Reported-by: default avatarMichael Thayer <michael.thayer@oracle.com>
Signed-off-by: default avatarThomas Gleixner <tglx@linutronix.de>
Cc: Michal Necasek <michal.necasek@oracle.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: knut.osmundsen@oracle.com
Cc: stable@vger.kernel.org
Cc: stern@rowland.harvard.edu
Cc: rt@linutronix.de
Link: http://lkml.kernel.org/r/20161022110552.253640125@linutronix.de


Signed-off-by: default avatarThomas Gleixner <tglx@linutronix.de>
parent 041ad7bc

kernel/time/timer.c

+10 −13
Original line number Diff line number Diff line
@@ -878,7 +878,7 @@ static inline struct timer_base *get_timer_base(u32 tflags) 


#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 *base, unsigned tflags) 


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 *base, unsigned tflags) 
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
@@ -1037,6 +1031,9 @@ __mod_timer(struct timer_list *timer, unsigned long expires, bool pending_only) 
		}

	}



	/* 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

CRA Git | Maintained and supported by SUSTech CRA and CCSE