lkml.org 
[lkml]   [1997]   [Sep]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Date
    SubjectTwo bugs in kernel time code
    The patch below is actually just an extract from my new PPSkit-0.3.0 that I'll
    announce a bit later.

    There's a very old bug in the kernel that rarely occurs (who's using NTP?):
    The code to update the CMOS clock does not consider the hour field. I had no
    clever idea to fix it, so I simply added a message to log the problem (well,
    a part of it).

    In adjtimex() several parameters were not checked, making it possibly to pass
    invalid values to the kernel. Also the plain old adjtim() was dependent on the
    time_status & STA_PLL, which should be completely independent of adjtimex()
    (ntp_adjtime).

    Especially it was allowed to change the read-only bits of time_status...

    The function now even returns TIME_ERROR if the flags in time_state indicate an
    error condition.

    It should go into 2.1 as well I guess...

    Ulrich


    diff -r -u --new-file /usr/src/linux-2.0.31-pre9/arch/i386/kernel/time.c
    /usr/src/linux/arch/i386/kernel/time.c
    --- /usr/src/linux-2.0.31-pre9/arch/i386/kernel/time.c Sun Nov 10 18:40:53 1996
    +++ /usr/src/linux/arch/i386/kernel/time.c Fri Sep 12 21:35:53 1997
    @@ -282,6 +286,9 @@
    * nowtime is written into the registers of the CMOS clock, it will
    * jump to the next second precisely 500 ms later. Check the Motorola
    * MC146818A or Dallas DS12887 data sheet for details.
    + *
    + * BUG: This routine does not handle hour overflow properly; it just
    + * sets the minutes. Usually you'll only notice that after reboot!
    */
    static int set_rtc_mmss(unsigned long nowtime)
    {
    @@ -318,8 +325,12 @@
    }
    CMOS_WRITE(real_seconds,RTC_SECONDS);
    CMOS_WRITE(real_minutes,RTC_MINUTES);
    - } else
    + } else {
    + printk(KERN_WARNING
    + "set_rtc_mmss: can't update from %d to %d\n",
    + cmos_minutes, real_minutes);
    retval = -1;
    + }

    /* The following flags have to be released exactly in this order,
    * otherwise the DS12887 (popular MC146818A clone with integrated
    diff -r -u --new-file /usr/src/linux-2.0.31-pre9/include/linux/timex.h
    /usr/src/linux/include/linux/timex.h
    --- /usr/src/linux-2.0.31-pre9/include/linux/timex.h Sat May 4 18:39:22 1996
    +++ /usr/src/linux/include/linux/timex.h Tue Sep 9 21:17:46 1997
    @@ -38,6 +38,8 @@
    * Derived linux/timex.h
    * 1995-08-13 Torsten Duwe
    * kernel PLL updated to 1994-12-13 specs (rfc-1589)
    + * 1997-08-30 Ulrich Windl
    + * Added new constant NTP_PHASE_LIMIT
    */
    #ifndef _LINUX_TIMEX_H
    #define _LINUX_TIMEX_H
    @@ -95,6 +97,7 @@
    #define MAXTIME (200L << PPS_AVG) /* max PPS error (jitter) (200 us) */
    #define MINSEC 16L /* min interval between updates (s) */
    #define MAXSEC 1200L /* max interval between updates (s) */
    +#define NTP_PHASE_LIMIT (MAXPHASE << 5) /* beyond max. dispersion */

    /*
    * The following defines are used only if a pulse-per-second (PPS)
    diff -r -u --new-file /usr/src/linux-2.0.31-pre9/kernel/time.c
    /usr/src/linux/kernel/time.c
    --- /usr/src/linux-2.0.31-pre9/kernel/time.c Tue Sep 9 19:09:49 1997
    +++ /usr/src/linux/kernel/time.c Sat Sep 13 19:48:48 1997
    @@ -195,14 +494,14 @@
    asmlinkage int sys_adjtimex(struct timex *txc_p)
    {
    long ltemp, mtemp, save_adjust;
    - int error;
    + int error = 0;

    /* Local copy of parameter */
    struct timex txc;

    error = verify_area(VERIFY_WRITE, txc_p, sizeof(struct timex));
    if (error)
    - return error;
    + return error; /* do not write results */

    /* Copy the user data space into the kernel copy
    * structure. But bear in mind that the structures
    @@ -216,112 +515,139 @@

    /* Now we validate the data before disabling interrupts
    */
    -
    if (txc.modes != ADJ_OFFSET_SINGLESHOT && (txc.modes & ADJ_OFFSET))
    - /* adjustment Offset limited to +- .512 seconds */
    - if (txc.offset <= - MAXPHASE || txc.offset >= MAXPHASE )
    - return -EINVAL;
    -
    - /* if the quartz is off by more than 10% something is VERY wrong ! */
    - if (txc.modes & ADJ_TICK)
    - if (txc.tick < 900000/HZ || txc.tick > 1100000/HZ)
    - return -EINVAL;
    + /* adjustment Offset limited to +- .512 seconds */
    + if (txc.offset <= - MAXPHASE || txc.offset >= MAXPHASE )
    + return -EINVAL;

    cli();

    - /* Save for later - semantics of adjtime is to return old value */
    + /* Save for later - semantics of adjtime() is to return old value */
    save_adjust = time_adjust;

    /* If there are input parameters, then process them */
    if (txc.modes)
    {
    - if (time_state == TIME_BAD)
    - time_state = TIME_OK;
    -
    - if (txc.modes & ADJ_STATUS)
    - time_status = txc.status;
    + if (time_state == TIME_ERROR)
    + time_state = TIME_OK; /* reset error -- why? */

    - if (txc.modes & ADJ_FREQUENCY)
    + if (txc.modes & ADJ_STATUS) /* only set allowed bits */
    + time_status = (txc.status & ~STA_RONLY) |
    + (time_status & STA_RONLY);
    +
    + if (txc.modes & ADJ_FREQUENCY) { /* p. 22 */
    + if (txc.freq > MAXFREQ || txc.freq < -MAXFREQ) {
    + error = -EINVAL;
    + goto leave;
    + }
    time_freq = txc.freq;
    + }

    - if (txc.modes & ADJ_MAXERROR)
    + if (txc.modes & ADJ_MAXERROR) {
    + if (txc.maxerror < 0 || txc.maxerror >= NTP_PHASE_LIMIT) {
    + error = -EINVAL;
    + goto leave;
    + }
    time_maxerror = txc.maxerror;
    + }

    - if (txc.modes & ADJ_ESTERROR)
    + if (txc.modes & ADJ_ESTERROR) {
    + if (txc.esterror < 0 || txc.esterror >= NTP_PHASE_LIMIT) {
    + error = -EINVAL;
    + goto leave;
    + }
    time_esterror = txc.esterror;
    + }

    - if (txc.modes & ADJ_TIMECONST)
    + if (txc.modes & ADJ_TIMECONST) { /* p. 24 */
    + if (txc.constant < 0 || txc.constant > MAXTC) {
    + error = -EINVAL;
    + goto leave;
    + }
    time_constant = txc.constant;
    + }

    - if (txc.modes & ADJ_OFFSET)
    - if ((txc.modes == ADJ_OFFSET_SINGLESHOT)
    - || !(time_status & STA_PLL))
    - {
    - time_adjust = txc.offset;
    - }
    - else if ((time_status & STA_PLL)||(time_status & STA_PPSTIME))
    - {
    - ltemp = (time_status & STA_PPSTIME &&
    - time_status & STA_PPSSIGNAL) ?
    - pps_offset : txc.offset;
    -
    - /*
    - * Scale the phase adjustment and
    - * clamp to the operating range.
    - */
    - if (ltemp > MAXPHASE)
    - time_offset = MAXPHASE << SHIFT_UPDATE;
    - else if (ltemp < -MAXPHASE)
    - time_offset = -(MAXPHASE << SHIFT_UPDATE);
    - else
    - time_offset = ltemp << SHIFT_UPDATE;
    -
    - /*
    - * Select whether the frequency is to be controlled and in which
    - * mode (PLL or FLL). Clamp to the operating range. Ugly
    - * multiply/divide should be replaced someday.
    - */
    -
    - if (time_status & STA_FREQHOLD || time_reftime == 0)
    + if (txc.modes & ADJ_OFFSET) { /* values checked earlier */
    + if (txc.modes == ADJ_OFFSET_SINGLESHOT) {
    + /* adjtime() is independent from ntp_adjtime() */
    + time_adjust = txc.offset;
    + }
    + else if ( time_status & (STA_PLL | STA_PPSTIME) ) {
    + ltemp = (time_status & (STA_PPSTIME | STA_PPSSIGNAL)) ==
    + (STA_PPSTIME | STA_PPSSIGNAL) ?
    + pps_offset : txc.offset;
    +
    + /*
    + * Scale the phase adjustment and
    + * clamp to the operating range.
    + */
    + if (ltemp > MAXPHASE)
    + time_offset = MAXPHASE << SHIFT_UPDATE;
    + else if (ltemp < -MAXPHASE)
    + time_offset = -(MAXPHASE << SHIFT_UPDATE);
    + else
    + time_offset = ltemp << SHIFT_UPDATE;
    +
    + /*
    + * Select whether the frequency is to be controlled
    + * and in which mode (PLL or FLL). Clamp to the operating
    + * range. Ugly multiply/divide should be replaced someday.
    + */
    +
    + if (time_status & STA_FREQHOLD || time_reftime == 0)
    + time_reftime = xtime.tv_sec;
    + mtemp = xtime.tv_sec - time_reftime;
    time_reftime = xtime.tv_sec;
    - mtemp = xtime.tv_sec - time_reftime;
    - time_reftime = xtime.tv_sec;
    - if (time_status & STA_FLL)
    - {
    - if (mtemp >= MINSEC)
    - {
    - ltemp = ((time_offset / mtemp) << (SHIFT_USEC -
    - SHIFT_UPDATE));
    - if (ltemp < 0)
    - time_freq -= -ltemp >> SHIFT_KH;
    - else
    - time_freq += ltemp >> SHIFT_KH;
    - }
    - }
    - else
    - {
    - if (mtemp < MAXSEC)
    - {
    - ltemp *= mtemp;
    - if (ltemp < 0)
    - time_freq -= -ltemp >> (time_constant +
    - time_constant + SHIFT_KF -
    - SHIFT_USEC);
    - else
    - time_freq += ltemp >> (time_constant +
    - time_constant + SHIFT_KF -
    - SHIFT_USEC);
    - }
    + if (time_status & STA_FLL) {
    + if (mtemp >= MINSEC) {
    + ltemp = (time_offset / mtemp) << (SHIFT_USEC -
    + SHIFT_UPDATE);
    + if (ltemp < 0)
    + time_freq -= -ltemp >> SHIFT_KH;
    + else
    + time_freq += ltemp >> SHIFT_KH;
    + } else /* calibration interval too short (p. 12) */
    + time_state = TIME_ERROR;
    + } else { /* PLL mode */
    + if (mtemp < MAXSEC) {
    + ltemp *= mtemp;
    + if (ltemp < 0)
    + time_freq -= -ltemp >> (time_constant +
    + time_constant +
    + SHIFT_KF - SHIFT_USEC);
    + else
    + time_freq += ltemp >> (time_constant +
    + time_constant +
    + SHIFT_KF - SHIFT_USEC);
    + } else /* calibration interval too long (p. 12) */
    + time_state = TIME_ERROR;
    }
    - if (time_freq > time_tolerance)
    - time_freq = time_tolerance;
    - else if (time_freq < -time_tolerance)
    - time_freq = -time_tolerance;
    + if (time_freq > time_tolerance)
    + time_freq = time_tolerance;
    + else if (time_freq < -time_tolerance)
    + time_freq = -time_tolerance;
    } /* STA_PLL || STA_PPSTIME */
    - if (txc.modes & ADJ_TICK)
    - tick = txc.tick;
    -
    - }
    + } /* txc.modes & ADJ_OFFSET */
    + if (txc.modes & ADJ_TICK) {
    + /* if the quartz is off by more than 10% something is
    + VERY wrong ! */
    + if (txc.tick < 900000/HZ || txc.tick > 1100000/HZ) {
    + error = -EINVAL;
    + goto leave;
    + }
    + tick = txc.tick;
    + }
    + } /* txc.modes */
    +leave: if (((time_status & STA_PPSSIGNAL) == 0
    + && (time_status & (STA_PPSFREQ|STA_PPSTIME)) != 0)
    + /* p. 24, (b) */
    + || ((time_status
    + & (STA_PPSTIME|STA_PPSJITTER)) == (STA_PPSTIME|STA_PPSJITTER))
    + /* p. 24, (c) */
    + || ((time_status & STA_PPSFREQ) != 0
    + && (time_status & (STA_PPSWANDER|STA_PPSERROR)) != 0))
    + /* p. 24, (d) */
    + time_state = TIME_ERROR;
    txc.offset = save_adjust;
    txc.freq = time_freq;
    txc.maxerror = time_maxerror;
    @@ -344,5 +670,5 @@
    sti();

    memcpy_tofs(txc_p, &txc, sizeof(struct timex));
    - return time_state;
    + return error ? error :time_state;
    }

    \
     
     \ /
      Last update: 2005-03-22 13:40    [W:0.036 / U:2.384 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site