Messages in this thread Patch in this message | | | Subject | [PATCH] sched: Fix 32bit race in sched_clock_remote() | From | Peter Zijlstra <> | Date | Fri, 05 Apr 2013 18:36:40 +0200 |
| |
Thomas spotted a nasty 32bit race in sched_clock_remote() after way too many hours of debugging weirdness.
What happens is that sched_clock_remote() does regular machine word reads of sched_clock_data::clock; this appears safe since we use cmpxchg64() to update the variable and any half-read value would trigger a retry.
Except we don't validate the new value 'val' in the same way! Thus we can propagate non-atomic read errors into the clock value.
Cc: Ingo Molnar <mingo@kernel.org> Cc: Steven Rostedt <rostedt@goodmis.org> Debugged-by: Thomas Gleixner <tglx@linutronix.de> Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl> --- kernel/sched/clock.c | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-)
diff --git a/kernel/sched/clock.c b/kernel/sched/clock.c index c685e31..7042ef7 100644 --- a/kernel/sched/clock.c +++ b/kernel/sched/clock.c @@ -170,6 +170,21 @@ static u64 sched_clock_local(struct sched_clock_data *scd) return clock; } +#ifndef CONFIG_64BIT +/* + * 32bit machines can't atomically read a u64 except using cmpxchg64() + */ +static inline u64 scd_read_clock(struct sched_clock_data *scd) +{ + return cmpxchg64(&scd->clock, 0, 0); +} +#else +static inline u64 scd_read_clock(struct sched_clock_data *scd) +{ + return scd->clock; +} +#endif + static u64 sched_clock_remote(struct sched_clock_data *scd) { struct sched_clock_data *my_scd = this_scd(); @@ -178,8 +193,8 @@ static u64 sched_clock_remote(struct sched_clock_data *scd) sched_clock_local(my_scd); again: - this_clock = my_scd->clock; - remote_clock = scd->clock; + this_clock = scd_clock_read(my_scd); + remote_clock = scd_clock_read(scd); /* * Use the opportunity that we have both locks
| |