time: Verify time values in adjtimex ADJ_SETOFFSET to avoid overflow
authorJohn Stultz <john.stultz@linaro.org>
Fri, 4 Dec 2015 03:09:31 +0000 (22:09 -0500)
committerJohn Stultz <john.stultz@linaro.org>
Fri, 11 Dec 2015 06:41:06 +0000 (22:41 -0800)
For adjtimex()'s ADJ_SETOFFSET, make sure the tv_usec value is
sane. We might multiply them later which can cause an overflow
and undefined behavior.

This patch introduces new helper functions to simplify the
checking code and adds comments to clarify

Orginally this patch was by Sasha Levin, but I've basically
rewritten it, so he should get credit for finding the issue
and I should get the blame for any mistakes made since.

Also, credit to Richard Cochran for the phrasing used in the
comment for what is considered valid here.

Cc: Sasha Levin <sasha.levin@oracle.com>
Cc: Richard Cochran <richardcochran@gmail.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Reported-by: Sasha Levin <sasha.levin@oracle.com>
Signed-off-by: John Stultz <john.stultz@linaro.org>
include/linux/time.h
kernel/time/ntp.c
kernel/time/timekeeping.c

index beebe3a02d43f5c527633cbd12af690c02e8288d..297f09f23896d2db41cd90a0cad37746e6ed77aa 100644 (file)
@@ -125,6 +125,32 @@ static inline bool timeval_valid(const struct timeval *tv)
 
 extern struct timespec timespec_trunc(struct timespec t, unsigned gran);
 
+/*
+ * Validates if a timespec/timeval used to inject a time offset is valid.
+ * Offsets can be postive or negative. The value of the timeval/timespec
+ * is the sum of its fields, but *NOTE*: the field tv_usec/tv_nsec must
+ * always be non-negative.
+ */
+static inline bool timeval_inject_offset_valid(const struct timeval *tv)
+{
+       /* We don't check the tv_sec as it can be positive or negative */
+
+       /* Can't have more microseconds then a second */
+       if (tv->tv_usec < 0 || tv->tv_usec >= USEC_PER_SEC)
+               return false;
+       return true;
+}
+
+static inline bool timespec_inject_offset_valid(const struct timespec *ts)
+{
+       /* We don't check the tv_sec as it can be positive or negative */
+
+       /* Can't have more nanoseconds then a second */
+       if (ts->tv_nsec < 0 || ts->tv_nsec >= NSEC_PER_SEC)
+               return false;
+       return true;
+}
+
 #define CURRENT_TIME           (current_kernel_time())
 #define CURRENT_TIME_SEC       ((struct timespec) { get_seconds(), 0 })
 
index 125fc034235531247d22618f2edc23bd40e5925b..4073c9550af94f4537e15f156abdfc95dfa2396c 100644 (file)
@@ -676,8 +676,14 @@ int ntp_validate_timex(struct timex *txc)
                        return -EINVAL;
        }
 
-       if ((txc->modes & ADJ_SETOFFSET) && (!capable(CAP_SYS_TIME)))
-               return -EPERM;
+       if (txc->modes & ADJ_SETOFFSET) {
+               /* In order to inject time, you gotta be super-user! */
+               if (!capable(CAP_SYS_TIME))
+                       return -EPERM;
+
+               if (!timeval_inject_offset_valid(&txc->time))
+                       return -EINVAL;
+       }
 
        /*
         * Check for potential multiplication overflows that can
index 99188ee5d9d0908df81d3acbadbc6bd1af44ef0f..d9249daf14ba8625304356331885c7674f67178b 100644 (file)
@@ -958,7 +958,7 @@ int timekeeping_inject_offset(struct timespec *ts)
        struct timespec64 ts64, tmp;
        int ret = 0;
 
-       if ((unsigned long)ts->tv_nsec >= NSEC_PER_SEC)
+       if (!timespec_inject_offset_valid(ts))
                return -EINVAL;
 
        ts64 = timespec_to_timespec64(*ts);