[patch] stime/settimeofday/adjtimex SMP races (2.2.12 and 2.3.18ac5)

Andrea Arcangeli (andrea@suse.de)
Fri, 17 Sep 1999 16:41:17 +0200 (CEST)


I reviewed the time-related stuff in 2.2.12 and I found potential SMP
races in the time-related syscalls. Basically the only problem is that the
time-syscalls are not protecting against each other. They are only
protecting themself against the timer irq. And this may lead to SMP races.

Actually I don't know if these SMP races may be the source of the RTC
screwedup with ntpd enabled (also considering that ntpd seems single
threaded...). Anyway here it is the fix for the races I spotted so far.
It's against 2.2.12 but it applyes cleanly to all 2.2.x 2.3.x kernels out
there as there aren't been NTP/time changes since 2.2.0. You may want to
try to reproduce the RTC SMP race with this fix applyed (I can't reproduce
in any way here, but I am not sure I configured xntp hard enough to
trigger the race).

--- 2.2.12/kernel/time.c Wed Mar 24 01:53:19 1999
+++ 2.2.12-ntp-SMP-races/kernel/time.c Fri Sep 17 16:29:11 1999
@@ -22,6 +22,8 @@
* "A Kernel Model for Precision Timekeeping" by Dave Mills
* Allow time_constant larger than MAXTC(6) for NTP v4 (MAXTC == 10)
* (Even though the technical memorandum forbids it)
+ * 1999-09-17 Andrea Arcangeli <andrea@suse.de>
+ * Fixed adjtimex/settimeofday/stime SMP races.
*/

#include <linux/mm.h>
@@ -76,6 +78,13 @@
return i;
}

+/* The xtime_lock is not only serializing the xtime read/writes but it's also
+ serializing all accesses to the global NTP variables.
+ NOTE NOTE: We really need a spinlock here as the global irq locking
+ only protect us against the timer irq and not against other time-related
+ syscall running under us. */
+extern rwlock_t xtime_lock;
+
/*
* sys_stime() can be implemented in user-level using
* sys_settimeofday(). Is this for backwards compatibility? If so,
@@ -91,14 +100,14 @@
return -EPERM;
if (get_user(value, tptr))
return -EFAULT;
- cli();
+ write_lock_irq(&xtime_lock);
xtime.tv_sec = value;
xtime.tv_usec = 0;
time_adjust = 0; /* stop active adjtime() */
time_status |= STA_UNSYNC;
time_maxerror = NTP_PHASE_LIMIT;
time_esterror = NTP_PHASE_LIMIT;
- sti();
+ write_unlock_irq(&xtime_lock);
return 0;
}

@@ -137,9 +146,9 @@
*/
inline static void warp_clock(void)
{
- cli();
+ write_lock_irq(&xtime_lock);
xtime.tv_sec += sys_tz.tz_minuteswest * 60;
- sti();
+ write_unlock_irq(&xtime_lock);
}

/*
@@ -220,7 +229,7 @@
int do_adjtimex(struct timex *txc)
{
long ltemp, mtemp, save_adjust;
- int result = time_state; /* mostly `TIME_OK' */
+ int result;

/* In order to modify anything, you gotta be super-user! */
if (txc->modes && !capable(CAP_SYS_TIME))
@@ -238,7 +247,8 @@
if (txc->tick < 900000/HZ || txc->tick > 1100000/HZ)
return -EINVAL;

- cli(); /* SMP: global cli() is enough protection. */
+ write_lock(&xtime_lock);
+ result = time_state; /* mostly `TIME_OK' */

/* Save for later - semantics of adjtime is to return old value */
save_adjust = time_adjust;
@@ -383,7 +393,6 @@
txc->constant = time_constant;
txc->precision = time_precision;
txc->tolerance = time_tolerance;
- do_gettimeofday(&txc->time);
txc->tick = tick;
txc->ppsfreq = pps_freq;
txc->jitter = pps_jitter >> PPS_AVG;
@@ -393,8 +402,8 @@
txc->calcnt = pps_calcnt;
txc->errcnt = pps_errcnt;
txc->stbcnt = pps_stbcnt;
-
- sti();
+ write_unlock_irq(&xtime_lock);
+ do_gettimeofday(&txc->time);
return(result);
}

Andrea

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/