[PATCH 0/2 v2] Fix for settimeofday() error checking regression

[PATCH 0/2 v2] Fix for settimeofday() error checking regression

[John Stultz]

As reported by Mika Westerberg here:
        https://lkml.org/lkml/2016/5/30/413

A regression in the settimeofday error checking snuck in via
86d3473224b0 ("time: Introduce do_sys_settimeofday64()"),
effecting cases where the timeval is null but the timezone
was set.

This patchset contains my proposed fix and an addition to
the kselftests to add checks for this case.


Thomas: Can you please queue the first patch in tip/timers/urgent?

Shuah: I'll leave the second patch to your discretion.


thanks
-john

v2:
* Add logic simplification suggested by Dima Stepanov
* Add tested by tag from Mika

Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
Cc: Baolin Wang <baolin.wang@linaro.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Shuah Khan <shuahkh@osg.samsung.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Richard Cochran <richardcochran@gmail.com>
Cc: Prarit Bhargava <prarit@redhat.com>

John Stultz (2):
  time: Fix problematic change in settimeofday error checking
  kselftests: timers: Add set-tz test case

 include/linux/timekeeping.h             |   3 +
 tools/testing/selftests/timers/Makefile |   3 +-
 tools/testing/selftests/timers/set-tz.c | 119 ++++++++++++++++++++++++++++++++
 3 files changed, 124 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/timers/set-tz.c

-- 
1.9.1

[PATCH 1/2 v2] time: Fix problematic change in settimeofday error checking

[John Stultz]

In commit 86d3473224b0 ("time: Introduce do_sys_settimeofday64()")
some of the checking for a valid timeval was subtley changed
which caused -EINVAL to be returned whenever the timeval was null.

However, it is possible to set the timezone data while specifying
a NULL timeval, which is usually done to handle systems where the
RTC keeps local time instead of UTC. Thus the patch causes such
systems to have the time incorrectly set.

This patch addresses the issue by handling the error conditionals
in the same way as was done previously.

Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
Cc: Baolin Wang <baolin.wang@linaro.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Shuah Khan <shuahkh@osg.samsung.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Richard Cochran <richardcochran@gmail.com>
Cc: Prarit Bhargava <prarit@redhat.com>
Tested-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Reported-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
v2: Add logic simplification from Dima Stepanov <dstepanov.src@gmail.com>

 include/linux/timekeeping.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/linux/timekeeping.h b/include/linux/timekeeping.h
index 37dbacf..816b754 100644
--- a/include/linux/timekeeping.h
+++ b/include/linux/timekeeping.h
@@ -21,6 +21,9 @@ static inline int do_sys_settimeofday(const struct timespec *tv,
 	struct timespec64 ts64;
 
 	if (!tv)
+		return do_sys_settimeofday64(NULL, tz);
+
+	if (!timespec_valid(tv))
 		return -EINVAL;
 
 	ts64 = timespec_to_timespec64(*tv);
-- 
1.9.1

[PATCH 2/2] kselftests: timers: Add set-tz test case

[John Stultz]

Mika Westerberg reported a erroneous change in the error
checking of settimeofday, so I wanted to add a test to ensure
we don't trip over this again.

Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
Cc: Baolin Wang <baolin.wang@linaro.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Shuah Khan <shuahkh@osg.samsung.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Richard Cochran <richardcochran@gmail.com>
Cc: Prarit Bhargava <prarit@redhat.com>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 tools/testing/selftests/timers/Makefile |   3 +-
 tools/testing/selftests/timers/set-tz.c | 119 ++++++++++++++++++++++++++++++++
 2 files changed, 121 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/timers/set-tz.c

diff --git a/tools/testing/selftests/timers/Makefile b/tools/testing/selftests/timers/Makefile
index 4a1be1b..1d55568 100644
--- a/tools/testing/selftests/timers/Makefile
+++ b/tools/testing/selftests/timers/Makefile
@@ -10,7 +10,7 @@ TEST_PROGS = posix_timers nanosleep nsleep-lat set-timer-lat mqueue-lat \
 
 TEST_PROGS_EXTENDED = alarmtimer-suspend valid-adjtimex adjtick change_skew \
 		      skew_consistency clocksource-switch leap-a-day \
-		      leapcrash set-tai set-2038
+		      leapcrash set-tai set-2038 set-tz
 
 bins = $(TEST_PROGS) $(TEST_PROGS_EXTENDED)
 
@@ -30,6 +30,7 @@ run_destructive_tests: run_tests
 	./clocksource-switch
 	./leap-a-day -s -i 10
 	./leapcrash
+	./set-tz
 	./set-tai
 	./set-2038
 
diff --git a/tools/testing/selftests/timers/set-tz.c b/tools/testing/selftests/timers/set-tz.c
new file mode 100644
index 0000000..f418492
--- /dev/null
+++ b/tools/testing/selftests/timers/set-tz.c
@@ -0,0 +1,119 @@
+/* Set tz value
+ *              by: John Stultz <john.stultz@linaro.org>
+ *              (C) Copyright Linaro 2016
+ *              Licensed under the GPLv2
+ *
+ *   This program is free software: you can redistribute it and/or modify
+ *   it under the terms of the GNU General Public License as published by
+ *   the Free Software Foundation, either version 2 of the License, or
+ *   (at your option) any later version.
+ *
+ *   This program is distributed in the hope that it will be useful,
+ *   but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *   GNU General Public License for more details.
+ */
+
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <time.h>
+#include <sys/time.h>
+#include <sys/timex.h>
+#include <string.h>
+#include <signal.h>
+#include <unistd.h>
+#ifdef KTEST
+#include "../kselftest.h"
+#else
+static inline int ksft_exit_pass(void)
+{
+	exit(0);
+}
+static inline int ksft_exit_fail(void)
+{
+	exit(1);
+}
+#endif
+
+int set_tz(int min, int dst)
+{
+	struct timezone tz;
+
+	tz.tz_minuteswest = min;
+	tz.tz_dsttime = dst;
+
+	return settimeofday(0, &tz);
+}
+
+int get_tz_min(void)
+{
+	struct timezone tz;
+	struct timeval tv;
+
+	memset(&tz, 0, sizeof(tz));
+	gettimeofday(&tv, &tz);
+	return tz.tz_minuteswest;
+}
+
+int get_tz_dst(void)
+{
+	struct timezone tz;
+	struct timeval tv;
+
+	memset(&tz, 0, sizeof(tz));
+	gettimeofday(&tv, &tz);
+	return tz.tz_dsttime;
+}
+
+int main(int argc, char **argv)
+{
+	int i, ret;
+	int min, dst;
+
+	min = get_tz_min();
+	dst = get_tz_dst();
+	printf("tz_minuteswest started at %i, dst at %i\n", min, dst);
+
+	printf("Checking tz_minuteswest can be properly set: ");
+	for (i = -15*60; i < 15*60; i += 30) {
+		ret = set_tz(i, dst);
+		ret = get_tz_min();
+		if (ret != i) {
+			printf("[FAILED] expected: %i got %i\n", i, ret);
+			goto err;
+		}
+	}
+	printf("[OK]\n");
+
+	printf("Checking invalid tz_minuteswest values are caught: ");
+
+	if (!set_tz(-15*60-1, dst)) {
+		printf("[FAILED] %i didn't return failure!\n", -15*60-1);
+		goto err;
+	}
+
+	if (!set_tz(15*60+1, dst)) {
+		printf("[FAILED] %i didn't return failure!\n", 15*60+1);
+		goto err;
+	}
+
+	if (!set_tz(-24*60, dst)) {
+		printf("[FAILED] %i didn't return failure!\n", -24*60);
+		goto err;
+	}
+
+	if (!set_tz(24*60, dst)) {
+		printf("[FAILED] %i didn't return failure!\n", 24*60);
+		goto err;
+	}
+
+	printf("[OK]\n");
+
+	set_tz(min, dst);
+	return ksft_exit_pass();
+
+err:
+	set_tz(min, dst);
+	return ksft_exit_fail();
+}
-- 
1.9.1

[tip:timers/urgent] time: Make settimeofday error checking work again

[tip-bot for John Stultz]

Commit-ID:  dfc2507b26af22b0bbc85251b8545b36d8bc5d72
Gitweb:     http://git.kernel.org/tip/dfc2507b26af22b0bbc85251b8545b36d8bc5d72
Author:     John Stultz <john.stultz@linaro.org>
AuthorDate: Wed, 1 Jun 2016 11:53:26 -0700
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Wed, 1 Jun 2016 21:13:43 +0200

time: Make settimeofday error checking work again

In commit 86d3473224b0 some of the checking for a valid timeval
was subtley changed which caused -EINVAL to be returned whenever
the timeval was null.

However, it is possible to set the timezone data while specifying
a NULL timeval, which is usually done to handle systems where the
RTC keeps local time instead of UTC. Thus the patch causes such
systems to have the time incorrectly set.

This patch addresses the issue by handling the error conditionals
in the same way as was done previously.

Fixes: 86d3473224b0 "time: Introduce do_sys_settimeofday64()"
Reported-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Signed-off-by: John Stultz <john.stultz@linaro.org>
Tested-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Cc: Prarit Bhargava <prarit@redhat.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Baolin Wang <baolin.wang@linaro.org>
Cc: Richard Cochran <richardcochran@gmail.com>
Cc: Shuah Khan <shuahkh@osg.samsung.com>
Link: http://lkml.kernel.org/r/1464807207-16530-2-git-send-email-john.stultz@linaro.org
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

---
 include/linux/timekeeping.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/linux/timekeeping.h b/include/linux/timekeeping.h
index 37dbacf..816b754 100644
--- a/include/linux/timekeeping.h
+++ b/include/linux/timekeeping.h
@@ -21,6 +21,9 @@ static inline int do_sys_settimeofday(const struct timespec *tv,
 	struct timespec64 ts64;
 
 	if (!tv)
+		return do_sys_settimeofday64(NULL, tz);
+
+	if (!timespec_valid(tv))
 		return -EINVAL;
 
 	ts64 = timespec_to_timespec64(*tv);

Re: [PATCH 2/2] kselftests: timers: Add set-tz test case

[Shuah Khan]

On 06/01/2016 12:53 PM, John Stultz wrote:
> Mika Westerberg reported a erroneous change in the error
> checking of settimeofday, so I wanted to add a test to ensure
> we don't trip over this again.
> 
> Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
> Cc: Baolin Wang <baolin.wang@linaro.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Shuah Khan <shuahkh@osg.samsung.com>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Richard Cochran <richardcochran@gmail.com>
> Cc: Prarit Bhargava <prarit@redhat.com>
> Signed-off-by: John Stultz <john.stultz@linaro.org>
> ---

Makes sense to get this into 4.7. Applied to linux-kselftest fixes.
I will get this into rc-2 or rc-3.

thanks,
-- Shuah