All of lore.kernel.org
 help / color / mirror / Atom feed
From: John Stultz <john.stultz@linaro.org>
To: lkml <linux-kernel@vger.kernel.org>
Cc: John Stultz <john.stultz@linaro.org>,
	Sasha Levin <sasha.levin@oracle.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@kernel.org>,
	Miroslav Lichvar <mlichvar@redhat.com>,
	Prarit Bhargava <prarit@redhat.com>,
	Richard Cochran <richardcochran@gmail.com>
Subject: [PATCH 07/11] time: Verify time values in adjtimex ADJ_SETOFFSET to avoid overflow
Date: Fri, 18 Dec 2015 13:39:12 -0800	[thread overview]
Message-ID: <1450474756-10144-8-git-send-email-john.stultz@linaro.org> (raw)
In-Reply-To: <1450474756-10144-1-git-send-email-john.stultz@linaro.org>

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: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Miroslav Lichvar <mlichvar@redhat.com>
Cc: Prarit Bhargava <prarit@redhat.com>
Cc: Richard Cochran <richardcochran@gmail.com>
Reported-by: Sasha Levin <sasha.levin@oracle.com>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 include/linux/time.h      | 26 ++++++++++++++++++++++++++
 kernel/time/ntp.c         | 10 ++++++++--
 kernel/time/timekeeping.c |  2 +-
 3 files changed, 35 insertions(+), 3 deletions(-)

diff --git a/include/linux/time.h b/include/linux/time.h
index beebe3a..297f09f 100644
--- a/include/linux/time.h
+++ b/include/linux/time.h
@@ -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 })
 
diff --git a/kernel/time/ntp.c b/kernel/time/ntp.c
index 125fc03..4073c95 100644
--- a/kernel/time/ntp.c
+++ b/kernel/time/ntp.c
@@ -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
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 99188ee..d9249da 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -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);
-- 
1.9.1


  parent reply	other threads:[~2015-12-18 21:39 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-12-18 21:39 [PATCH 00/11][GIT PULL] Timekeeping items for 4.5 John Stultz
2015-12-18 21:39 ` [PATCH 01/11] MAINTAINERS: Add entry for kernel/time/alarmtimer.c John Stultz
2015-12-18 21:39 ` [PATCH 02/11] alarmtimer: Avoid unexpected rtc interrupt when system resume from S3 John Stultz
2015-12-18 21:39 ` [PATCH 03/11] time: Avoid signed overflow in timekeeping_get_ns() John Stultz
2015-12-18 21:39 ` [PATCH 04/11] clocksource: Add CPU info to clocksource watchdog reporting John Stultz
2015-12-18 21:39 ` [PATCH 05/11] selftests/timers: fix write return value handlng John Stultz
2015-12-18 21:39 ` [PATCH 06/11] ntp: Verify offset doesn't overflow in ntp_update_offset John Stultz
2015-12-18 21:39 ` John Stultz [this message]
2015-12-18 21:39 ` [PATCH 08/11] timekeeping: Provide internal function __ktime_get_real_seconds John Stultz
2015-12-18 21:39 ` [PATCH 09/11] ntp: Change time_reftime to time64_t and utilize 64bit __ktime_get_real_seconds John Stultz
2015-12-18 21:39 ` [PATCH 10/11] ntp: Fix second_overflow's input parameter type to be 64bits John Stultz
2015-12-18 21:39 ` [PATCH 11/11] timekeeping: Cap adjustments so they don't exceed the maxadj value John Stultz

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1450474756-10144-8-git-send-email-john.stultz@linaro.org \
    --to=john.stultz@linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=mlichvar@redhat.com \
    --cc=prarit@redhat.com \
    --cc=richardcochran@gmail.com \
    --cc=sasha.levin@oracle.com \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.