[LTP] [PATCH] syscalls: clock_settime: Add test around y2038 vulnerability

[LTP] [PATCH] syscalls: clock_settime: Add test around y2038 vulnerability

[Viresh Kumar]

This adds a test around the y2038 vulnerability, it sets the system time
to just before y2038 time (i.e. max value that can be stored in s32),
and adds a timer to expire just after crossing it.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
Arnd/Cyril,

There is some problem with this test, I am trying to fire the timer
after 4 seconds, but it fires in no time. I am not sure on what's
incorrect here.

 runtest/syscalls                              |  1 +
 .../kernel/syscalls/clock_settime/.gitignore  |  1 +
 .../syscalls/clock_settime/clock_settime03.c  | 96 +++++++++++++++++++
 3 files changed, 98 insertions(+)
 create mode 100644 testcases/kernel/syscalls/clock_settime/clock_settime03.c

diff --git a/runtest/syscalls b/runtest/syscalls
index 8457db34e999..d7c3cbed611a 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -101,6 +101,7 @@ leapsec01 leapsec01
 
 clock_settime01 clock_settime01
 clock_settime02 clock_settime02
+clock_settime03 clock_settime03
 
 clone01 clone01
 clone02 clone02
diff --git a/testcases/kernel/syscalls/clock_settime/.gitignore b/testcases/kernel/syscalls/clock_settime/.gitignore
index 28121755006b..b66169b3eb7b 100644
--- a/testcases/kernel/syscalls/clock_settime/.gitignore
+++ b/testcases/kernel/syscalls/clock_settime/.gitignore
@@ -1,2 +1,3 @@
 clock_settime01
 clock_settime02
+clock_settime03
diff --git a/testcases/kernel/syscalls/clock_settime/clock_settime03.c b/testcases/kernel/syscalls/clock_settime/clock_settime03.c
new file mode 100644
index 000000000000..94e2d4ce4b9e
--- /dev/null
+++ b/testcases/kernel/syscalls/clock_settime/clock_settime03.c
@@ -0,0 +1,96 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2020 Linaro Limited. All rights reserved.
+ * Author: Viresh Kumar<viresh.kumar@linaro.org>
+ *
+ * Check Year 2038 related vulnerabilities.
+ */
+
+#include <signal.h>
+#include "config.h"
+#include "tst_timer.h"
+#include "tst_safe_clocks.h"
+
+#define EXPIREDELTA 4
+
+static struct tst_ts ts;
+static struct tst_its its;
+
+static struct test_variants {
+	int (*clock_settime)(clockid_t clk_id, void *ts);
+	int (*timer_settime)(timer_t timerid, int flags, void *its,
+			     void *old_its);
+	enum tst_ts_type type;
+	char *desc;
+} variants[] = {
+#if (__NR_clock_settime != __LTP__NR_INVALID_SYSCALL)
+	{ .clock_settime = sys_clock_settime, .timer_settime = sys_timer_settime, .type = TST_KERN_OLD_TIMESPEC, .desc = "syscall with old kernel spec"},
+#endif
+
+#if (__NR_clock_settime64 != __LTP__NR_INVALID_SYSCALL)
+	{ .clock_settime = sys_clock_settime64, .timer_settime = sys_timer_settime64, .type = TST_KERN_TIMESPEC, .desc = "syscall time64 with kernel spec"},
+#endif
+};
+
+static void setup(void)
+{
+	tst_res(TINFO, "Testing variant: %s", variants[tst_variant].desc);
+	ts.type = its.type = variants[tst_variant].type;
+}
+
+static void run(void)
+{
+	struct test_variants *tv = &variants[tst_variant];
+	unsigned long long time = 0x7FFFFFFE; /* Time just before y2038 */
+	struct sigevent ev = {
+		.sigev_notify = SIGEV_SIGNAL,
+		.sigev_signo = SIGABRT,
+	};
+	timer_t timer;
+	sigset_t set;
+	int sig, ret;
+
+	if (sigemptyset(&set) == -1)
+		tst_brk(TBROK, "sigemptyset() failed");
+
+	if (sigaddset(&set, SIGABRT) == -1)
+		tst_brk(TBROK, "sigaddset() failed");
+
+	if (sigprocmask(SIG_BLOCK, &set, NULL) == -1)
+		tst_brk(TBROK, "sigprocmask() failed");
+
+	TEST(tst_syscall(__NR_timer_create, CLOCK_REALTIME_ALARM, &ev, &timer));
+	if (TST_RET != 0)
+		tst_brk(TBROK | TERRNO, "timer_create() failed");
+
+	tst_ts_set_sec(&ts, time);
+	tst_ts_set_nsec(&ts, 0);
+
+	ret = tv->clock_settime(CLOCK_REALTIME, tst_ts_get(&ts));
+	if (ret == -1)
+		tst_brk(TBROK | TERRNO, "clock_settime() failed");
+
+	tst_its_set_time(&its, time + EXPIREDELTA, 0, 0, 0);
+
+	TEST(tv->timer_settime(timer, TIMER_ABSTIME, tst_its_get(&its), NULL));
+	if (TST_RET == -1)
+		tst_brk(TBROK | TTERRNO, "timer_settime() failed");
+
+	if (sigwait(&set, &sig) == -1)
+		tst_brk(TBROK, "sigwait() failed");
+
+	if (sig == SIGABRT) {
+		tst_res(TPASS, "clock_settime(): Y2038 test passed");
+		return;
+	}
+
+	tst_res(TFAIL, "clock_settime(): Y2038 test failed");
+}
+
+static struct tst_test test = {
+	.test_all = run,
+	.test_variants = ARRAY_SIZE(variants),
+	.setup = setup,
+	.needs_root = 1,
+	.restore_wallclock = 1,
+};
-- 
2.25.0.rc1.19.g042ed3e048af

[LTP] [PATCH] syscalls: clock_settime: Add test around y2038 vulnerability

[Arnd Bergmann]

On Tue, May 26, 2020 at 2:10 PM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> This adds a test around the y2038 vulnerability, it sets the system time
> to just before y2038 time (i.e. max value that can be stored in s32),
> and adds a timer to expire just after crossing it.
>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>

Nice!
> +static struct test_variants {
> +       int (*clock_settime)(clockid_t clk_id, void *ts);
> +       int (*timer_settime)(timer_t timerid, int flags, void *its,
> +                            void *old_its);
> +       enum tst_ts_type type;
> +       char *desc;
> +} variants[] = {
> +#if (__NR_clock_settime != __LTP__NR_INVALID_SYSCALL)
> +       { .clock_settime = sys_clock_settime, .timer_settime = sys_timer_settime, .type = TST_KERN_OLD_TIMESPEC, .desc = "syscall with old kernel spec"},
> +#endif
> +
> +#if (__NR_clock_settime64 != __LTP__NR_INVALID_SYSCALL)
> +       { .clock_settime = sys_clock_settime64, .timer_settime = sys_timer_settime64, .type = TST_KERN_TIMESPEC, .desc = "syscall time64 with kernel spec"},
> +#endif

I think the first one has to be guarded so we do not try to set
the time to just before the end with sys_clock_settime() on
32-bit machines, as kernels that don't fully support 64-bit
time_t behave in unpredictable ways when you do that and
are likely to crash.

However, we probably do want to test this on 64-bit kernels
with sys_clock_settime() anyway.

> +       tst_ts_set_sec(&ts, time);
> +       tst_ts_set_nsec(&ts, 0);
> +
> +       ret = tv->clock_settime(CLOCK_REALTIME, tst_ts_get(&ts));
> +       if (ret == -1)
> +               tst_brk(TBROK | TERRNO, "clock_settime() failed");
> +
> +       tst_its_set_time(&its, time + EXPIREDELTA, 0, 0, 0);

I suspect this is where it fails for the 32-bit time_t case, as the expiration
date for timer_settime wraps to year 1902.

     Arnd

[LTP] [PATCH V2] syscalls: clock_settime: Add test around y2038 vulnerability

[Viresh Kumar]

This adds a test around the y2038 vulnerability, it sets the system time
to just before y2038 time (i.e. max value that can be stored in s32),
and adds a timer to expire just after crossing it.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
V2:
- Fix the y2038 bug in the test itself, changes in the setup() routine.
  :)

 runtest/syscalls                              |   1 +
 .../kernel/syscalls/clock_settime/.gitignore  |   1 +
 .../syscalls/clock_settime/clock_settime03.c  | 122 ++++++++++++++++++
 3 files changed, 124 insertions(+)
 create mode 100644 testcases/kernel/syscalls/clock_settime/clock_settime03.c

diff --git a/runtest/syscalls b/runtest/syscalls
index 8457db34e999..d7c3cbed611a 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -101,6 +101,7 @@ leapsec01 leapsec01
 
 clock_settime01 clock_settime01
 clock_settime02 clock_settime02
+clock_settime03 clock_settime03
 
 clone01 clone01
 clone02 clone02
diff --git a/testcases/kernel/syscalls/clock_settime/.gitignore b/testcases/kernel/syscalls/clock_settime/.gitignore
index 28121755006b..b66169b3eb7b 100644
--- a/testcases/kernel/syscalls/clock_settime/.gitignore
+++ b/testcases/kernel/syscalls/clock_settime/.gitignore
@@ -1,2 +1,3 @@
 clock_settime01
 clock_settime02
+clock_settime03
diff --git a/testcases/kernel/syscalls/clock_settime/clock_settime03.c b/testcases/kernel/syscalls/clock_settime/clock_settime03.c
new file mode 100644
index 000000000000..9e316378b1cc
--- /dev/null
+++ b/testcases/kernel/syscalls/clock_settime/clock_settime03.c
@@ -0,0 +1,122 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2020 Linaro Limited. All rights reserved.
+ * Author: Viresh Kumar<viresh.kumar@linaro.org>
+ *
+ * Check Year 2038 related vulnerabilities.
+ */
+
+#include <signal.h>
+#include "config.h"
+#include "tst_timer.h"
+#include "tst_safe_clocks.h"
+
+#define EXPIREDELTA 3
+
+static struct tst_ts ts;
+static struct tst_its its;
+
+static struct test_variants {
+	int (*clock_gettime)(clockid_t clk_id, void *ts);
+	int (*clock_settime)(clockid_t clk_id, void *ts);
+	int (*timer_settime)(timer_t timerid, int flags, void *its,
+			     void *old_its);
+	enum tst_ts_type type;
+	char *desc;
+} variants[] = {
+#if (__NR_clock_settime != __LTP__NR_INVALID_SYSCALL)
+	{ .clock_gettime = sys_clock_gettime, .clock_settime = sys_clock_settime, .timer_settime = sys_timer_settime, .type = TST_KERN_OLD_TIMESPEC, .desc = "syscall with old kernel spec"},
+#endif
+
+#if (__NR_clock_settime64 != __LTP__NR_INVALID_SYSCALL)
+	{ .clock_gettime = sys_clock_gettime64, .clock_settime = sys_clock_settime64, .timer_settime = sys_timer_settime64, .type = TST_KERN_TIMESPEC, .desc = "syscall time64 with kernel spec"},
+#endif
+};
+
+static void setup(void)
+{
+	struct test_variants *tv = &variants[tst_variant];
+	unsigned long long time = 0x7FFFFFFF; /* Time just before y2038 */
+	struct tst_ts end;
+	int ret;
+
+	tst_res(TINFO, "Testing variant: %s", tv->desc);
+	ts.type = end.type = its.type = tv->type;
+
+	/* Check if the kernel is y2038 safe */
+	if (tv->type != TST_KERN_OLD_TIMESPEC)
+		return;
+
+	tst_ts_set_sec(&ts, time);
+	tst_ts_set_nsec(&ts, 0);
+
+	ret = tv->clock_settime(CLOCK_REALTIME, tst_ts_get(&ts));
+	if (ret == -1)
+		tst_brk(TBROK | TERRNO, "clock_settime() failed");
+
+	usleep(2000);
+
+	ret = tv->clock_gettime(CLOCK_REALTIME, tst_ts_get(&end));
+	if (ret == -1)
+		tst_brk(TBROK | TERRNO, "clock_gettime() failed");
+
+	if (tst_ts_lt(end, ts))
+		tst_brk(TFAIL, "Kernel isn't Year 2038 safe, abandon test");
+}
+
+static void run(void)
+{
+	struct test_variants *tv = &variants[tst_variant];
+	unsigned long long time = 0x7FFFFFFE; /* Time just before y2038 */
+	struct sigevent ev = {
+		.sigev_notify = SIGEV_SIGNAL,
+		.sigev_signo = SIGABRT,
+	};
+	timer_t timer;
+	sigset_t set;
+	int sig, ret;
+
+	if (sigemptyset(&set) == -1)
+		tst_brk(TBROK, "sigemptyset() failed");
+
+	if (sigaddset(&set, SIGABRT) == -1)
+		tst_brk(TBROK, "sigaddset() failed");
+
+	if (sigprocmask(SIG_BLOCK, &set, NULL) == -1)
+		tst_brk(TBROK, "sigprocmask() failed");
+
+	TEST(tst_syscall(__NR_timer_create, CLOCK_REALTIME_ALARM, &ev, &timer));
+	if (TST_RET != 0)
+		tst_brk(TBROK | TERRNO, "timer_create() failed");
+
+	tst_ts_set_sec(&ts, time);
+	tst_ts_set_nsec(&ts, 0);
+
+	ret = tv->clock_settime(CLOCK_REALTIME, tst_ts_get(&ts));
+	if (ret == -1)
+		tst_brk(TBROK | TERRNO, "clock_settime() failed");
+
+	tst_its_set_time(&its, time + EXPIREDELTA, 0, 0, 0);
+
+	TEST(tv->timer_settime(timer, TIMER_ABSTIME, tst_its_get(&its), NULL));
+	if (TST_RET == -1)
+		tst_brk(TBROK | TTERRNO, "timer_settime() failed");
+
+	if (sigwait(&set, &sig) == -1)
+		tst_brk(TBROK, "sigwait() failed");
+
+	if (sig == SIGABRT) {
+		tst_res(TPASS, "clock_settime(): Y2038 test passed");
+		return;
+	}
+
+	tst_res(TFAIL, "clock_settime(): Y2038 test failed");
+}
+
+static struct tst_test test = {
+	.test_all = run,
+	.test_variants = ARRAY_SIZE(variants),
+	.setup = setup,
+	.needs_root = 1,
+	.restore_wallclock = 1,
+};
-- 
2.25.0.rc1.19.g042ed3e048af

[LTP] [PATCH] syscalls: clock_settime: Add test around y2038 vulnerability

[Viresh Kumar]

On 27-05-20, 13:18, Arnd Bergmann wrote:
> On Tue, May 26, 2020 at 2:10 PM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> >
> > This adds a test around the y2038 vulnerability, it sets the system time
> > to just before y2038 time (i.e. max value that can be stored in s32),
> > and adds a timer to expire just after crossing it.
> >
> > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> 
> Nice!
> > +static struct test_variants {
> > +       int (*clock_settime)(clockid_t clk_id, void *ts);
> > +       int (*timer_settime)(timer_t timerid, int flags, void *its,
> > +                            void *old_its);
> > +       enum tst_ts_type type;
> > +       char *desc;
> > +} variants[] = {
> > +#if (__NR_clock_settime != __LTP__NR_INVALID_SYSCALL)
> > +       { .clock_settime = sys_clock_settime, .timer_settime = sys_timer_settime, .type = TST_KERN_OLD_TIMESPEC, .desc = "syscall with old kernel spec"},
> > +#endif
> > +
> > +#if (__NR_clock_settime64 != __LTP__NR_INVALID_SYSCALL)
> > +       { .clock_settime = sys_clock_settime64, .timer_settime = sys_timer_settime64, .type = TST_KERN_TIMESPEC, .desc = "syscall time64 with kernel spec"},
> > +#endif
> 
> I think the first one has to be guarded so we do not try to set
> the time to just before the end with sys_clock_settime() on
> 32-bit machines, as kernels that don't fully support 64-bit
> time_t behave in unpredictable ways when you do that and
> are likely to crash.
> 
> However, we probably do want to test this on 64-bit kernels
> with sys_clock_settime() anyway.

I have done this differently, please see V2.

-- 
viresh

[LTP] [PATCH V2] syscalls: clock_settime: Add test around y2038 vulnerability

[Arnd Bergmann]

On Thu, May 28, 2020 at 8:57 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> This adds a test around the y2038 vulnerability, it sets the system time
> to just before y2038 time (i.e. max value that can be stored in s32),
> and adds a timer to expire just after crossing it.
>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
> V2:
> - Fix the y2038 bug in the test itself, changes in the setup() routine.
>   :)

So I assume this version works as expected?
I don't see any more problems with it.

> +       ret = tv->clock_settime(CLOCK_REALTIME, tst_ts_get(&ts));
> +       if (ret == -1)
> +               tst_brk(TBROK | TERRNO, "clock_settime() failed");
> +
> +       tst_its_set_time(&its, time + EXPIREDELTA, 0, 0, 0);
> +
> +       TEST(tv->timer_settime(timer, TIMER_ABSTIME, tst_its_get(&its), NULL));
> +       if (TST_RET == -1)
> +               tst_brk(TBROK | TTERRNO, "timer_settime() failed");
> +
> +       if (sigwait(&set, &sig) == -1)
> +               tst_brk(TBROK, "sigwait() failed");

Should you maybe check the time after the expiration to ensure the
timer ran for the expected length?

I suppose you can read the time in CLOCK_MONOTONIC to check
for the elapsed time regardless of what the kernel might think the
CLOCK_REALTIME is after this.

      Arnd

[LTP] [PATCH V2] syscalls: clock_settime: Add test around y2038 vulnerability

[Viresh Kumar]

On 28-05-20, 10:27, Arnd Bergmann wrote:
> On Thu, May 28, 2020 at 8:57 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> >
> > This adds a test around the y2038 vulnerability, it sets the system time
> > to just before y2038 time (i.e. max value that can be stored in s32),
> > and adds a timer to expire just after crossing it.
> >
> > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> > ---
> > V2:
> > - Fix the y2038 bug in the test itself, changes in the setup() routine.
> >   :)
> 
> So I assume this version works as expected?
> I don't see any more problems with it.

Absolutely perfectly. I don't know what the problem with timer was
earlier, maybe it was related to a different way I was trying to
capture the signal with (i.e. SAFE_SIGNAL(SIGALRM, sighandler)).

But this (and the earlier patch as well, perhaps I tested even an
earlier version then) works just fine.

> > +       ret = tv->clock_settime(CLOCK_REALTIME, tst_ts_get(&ts));
> > +       if (ret == -1)
> > +               tst_brk(TBROK | TERRNO, "clock_settime() failed");

But I noticed that even this version may not be good enough, as I am
still doing the same thing in setup(), i.e. setting time to just
before y2038 to test if it is y2038 safe or not. I believe even that
isn't fine ?

> > +
> > +       tst_its_set_time(&its, time + EXPIREDELTA, 0, 0, 0);
> > +
> > +       TEST(tv->timer_settime(timer, TIMER_ABSTIME, tst_its_get(&its), NULL));
> > +       if (TST_RET == -1)
> > +               tst_brk(TBROK | TTERRNO, "timer_settime() failed");
> > +
> > +       if (sigwait(&set, &sig) == -1)
> > +               tst_brk(TBROK, "sigwait() failed");
> 
> Should you maybe check the time after the expiration to ensure the
> timer ran for the expected length?
> 
> I suppose you can read the time in CLOCK_MONOTONIC to check
> for the elapsed time regardless of what the kernel might think the
> CLOCK_REALTIME is after this.

This should be enough I believe.

diff --git a/testcases/kernel/syscalls/clock_settime/clock_settime03.c b/testcases/kernel/syscalls/clock_settime/clock_settime03.c
index 9e316378b1cc..876651a5d537 100644
--- a/testcases/kernel/syscalls/clock_settime/clock_settime03.c
+++ b/testcases/kernel/syscalls/clock_settime/clock_settime03.c
@@ -13,7 +13,7 @@
 
 #define EXPIREDELTA 3
 
-static struct tst_ts ts;
+static struct tst_ts ts, end;
 static struct tst_its its;
 
 static struct test_variants {
@@ -37,7 +37,6 @@ static void setup(void)
 {
        struct test_variants *tv = &variants[tst_variant];
        unsigned long long time = 0x7FFFFFFF; /* Time just before y2038 */
-       struct tst_ts end;
        int ret;
 
        tst_res(TINFO, "Testing variant: %s", tv->desc);
@@ -72,6 +71,7 @@ static void run(void)
                .sigev_notify = SIGEV_SIGNAL,
                .sigev_signo = SIGABRT,
        };
+       struct tst_ts diff;
        timer_t timer;
        sigset_t set;
        int sig, ret;
@@ -105,7 +105,16 @@ static void run(void)
        if (sigwait(&set, &sig) == -1)
                tst_brk(TBROK, "sigwait() failed");
 
+       ret = tv->clock_gettime(CLOCK_REALTIME, tst_ts_get(&end));
+       if (ret == -1)
+               tst_brk(TBROK | TERRNO, "clock_gettime() failed");
+
        if (sig == SIGABRT) {
+               diff = tst_ts_diff(end, ts);
+
+               if (tst_ts_get_sec(diff) != EXPIREDELTA)
+                       tst_res(TINFO, "Test slept longer than it should have, expected:%d, actual:%lld",
+                               EXPIREDELTA, tst_ts_get_sec(diff));
                tst_res(TPASS, "clock_settime(): Y2038 test passed");
                return;
        }

-- 
viresh

[LTP] [PATCH V2] syscalls: clock_settime: Add test around y2038 vulnerability

[Arnd Bergmann]

On Thu, May 28, 2020 at 11:11 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> On 28-05-20, 10:27, Arnd Bergmann wrote:
> > On Thu, May 28, 2020 at 8:57 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> >
> > > +       ret = tv->clock_settime(CLOCK_REALTIME, tst_ts_get(&ts));
> > > +       if (ret == -1)
> > > +               tst_brk(TBROK | TERRNO, "clock_settime() failed");
>
> But I noticed that even this version may not be good enough, as I am
> still doing the same thing in setup(), i.e. setting time to just
> before y2038 to test if it is y2038 safe or not. I believe even that
> isn't fine ?

Good point. I see you have this check above that:

+       /* Check if the kernel is y2038 safe */
+       if (tv->type != TST_KERN_OLD_TIMESPEC)
+               return;
+

So that part should prevent it from doing something bad, but I now
see it's still not great because it also prevents the test case from running
on 64-bit architectures. If you change the condition to also check for
sizeof(__kernel_old_timespec) it should be ok.

> diff --git a/testcases/kernel/syscalls/clock_settime/clock_settime03.c b/testcases/kernel/syscalls/clock_settime/clock_settime03.c
> index 9e316378b1cc..876651a5d537 100644
> --- a/testcases/kernel/syscalls/clock_settime/clock_settime03.c
> +++ b/testcases/kernel/syscalls/clock_settime/clock_settime03.c
> @@ -72,6 +71,7 @@ static void run(void)
>                 .sigev_notify = SIGEV_SIGNAL,
>                 .sigev_signo = SIGABRT,
>         };
> +       struct tst_ts diff;
>         timer_t timer;
>         sigset_t set;
>         int sig, ret;
> @@ -105,7 +105,16 @@ static void run(void)
>         if (sigwait(&set, &sig) == -1)
>                 tst_brk(TBROK, "sigwait() failed");
>
> +       ret = tv->clock_gettime(CLOCK_REALTIME, tst_ts_get(&end));
> +       if (ret == -1)
> +               tst_brk(TBROK | TERRNO, "clock_gettime() failed");
> +
>         if (sig == SIGABRT) {
> +               diff = tst_ts_diff(end, ts);
> +
> +               if (tst_ts_get_sec(diff) != EXPIREDELTA)
> +                       tst_res(TINFO, "Test slept longer than it should have, expected:%d, actual:%lld",
> +                               EXPIREDELTA, tst_ts_get_sec(diff));
>                 tst_res(TPASS, "clock_settime(): Y2038 test passed");
>                 return;

Yes, I think that should print a warning when something goes wrong,
but it can be improved:

- don't say it slept longer when it could also have slept shorter, or not
  slept at all. It's probably enough to say that the expired time is not what
  was expected and leave the interpretation to a person

- comparing only the seconds means that you warn when the elapsed
  time was less than expected or more than 1000000000 nanoseconds
  longer than expected, but that is a fairly long and arbitrary interval.
  Maybe make it something like 50ms and use a constant for it, so it
  can be adjusted if necessary.

        Arnd

[LTP] [PATCH V2] syscalls: clock_settime: Add test around y2038 vulnerability

[Viresh Kumar]

On 28-05-20, 11:31, Arnd Bergmann wrote:
> On Thu, May 28, 2020 at 11:11 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> >
> > On 28-05-20, 10:27, Arnd Bergmann wrote:
> > > On Thu, May 28, 2020 at 8:57 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> > >
> > > > +       ret = tv->clock_settime(CLOCK_REALTIME, tst_ts_get(&ts));
> > > > +       if (ret == -1)
> > > > +               tst_brk(TBROK | TERRNO, "clock_settime() failed");
> >
> > But I noticed that even this version may not be good enough, as I am
> > still doing the same thing in setup(), i.e. setting time to just
> > before y2038 to test if it is y2038 safe or not. I believe even that
> > isn't fine ?
> 
> Good point. I see you have this check above that:
> 
> +       /* Check if the kernel is y2038 safe */
> +       if (tv->type != TST_KERN_OLD_TIMESPEC)
> +               return;
> +
> 
> So that part should prevent it from doing something bad,

Not really. That code is part of the setup() routine and with "return"
we will go and run the actual test run(). That code is there to avoid
running the code on time64 implementation unnecessarily.

> but I now
> see it's still not great because it also prevents the test case from running
> on 64-bit architectures. If you change the condition to also check for
> sizeof(__kernel_old_timespec) it should be ok.

What about this instead of the whole setup() changes:

       /* Check if the kernel is y2038 safe */
       if (tv->type == TST_KERN_OLD_TIMESPEC &&
           sizeof(__kernel_old_timespec) == 8)
               tst_brk("Invalid configuration");

> > diff --git a/testcases/kernel/syscalls/clock_settime/clock_settime03.c b/testcases/kernel/syscalls/clock_settime/clock_settime03.c
> > index 9e316378b1cc..876651a5d537 100644
> > --- a/testcases/kernel/syscalls/clock_settime/clock_settime03.c
> > +++ b/testcases/kernel/syscalls/clock_settime/clock_settime03.c
> > @@ -72,6 +71,7 @@ static void run(void)
> >                 .sigev_notify = SIGEV_SIGNAL,
> >                 .sigev_signo = SIGABRT,
> >         };
> > +       struct tst_ts diff;
> >         timer_t timer;
> >         sigset_t set;
> >         int sig, ret;
> > @@ -105,7 +105,16 @@ static void run(void)
> >         if (sigwait(&set, &sig) == -1)
> >                 tst_brk(TBROK, "sigwait() failed");
> >
> > +       ret = tv->clock_gettime(CLOCK_REALTIME, tst_ts_get(&end));
> > +       if (ret == -1)
> > +               tst_brk(TBROK | TERRNO, "clock_gettime() failed");
> > +
> >         if (sig == SIGABRT) {
> > +               diff = tst_ts_diff(end, ts);
> > +
> > +               if (tst_ts_get_sec(diff) != EXPIREDELTA)
> > +                       tst_res(TINFO, "Test slept longer than it should have, expected:%d, actual:%lld",
> > +                               EXPIREDELTA, tst_ts_get_sec(diff));
> >                 tst_res(TPASS, "clock_settime(): Y2038 test passed");
> >                 return;
> 
> Yes, I think that should print a warning when something goes wrong,
> but it can be improved:
> 
> - don't say it slept longer when it could also have slept shorter, or not
>   slept at all. It's probably enough to say that the expired time is not what
>   was expected and leave the interpretation to a person

Right.

> - comparing only the seconds means that you warn when the elapsed
>   time was less than expected or more than 1000000000 nanoseconds
>   longer than expected, but that is a fairly long and arbitrary interval.
>   Maybe make it something like 50ms and use a constant for it, so it
>   can be adjusted if necessary.

Ok.

-- 
viresh

[LTP] [PATCH V2] syscalls: clock_settime: Add test around y2038 vulnerability

[Arnd Bergmann]

On Thu, May 28, 2020 at 12:35 PM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> On 28-05-20, 11:31, Arnd Bergmann wrote:
> > On Thu, May 28, 2020 at 11:11 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> > >
> > > On 28-05-20, 10:27, Arnd Bergmann wrote:
> > > > On Thu, May 28, 2020 at 8:57 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> > > >
> > > > > +       ret = tv->clock_settime(CLOCK_REALTIME, tst_ts_get(&ts));
> > > > > +       if (ret == -1)
> > > > > +               tst_brk(TBROK | TERRNO, "clock_settime() failed");
> > >
> > > But I noticed that even this version may not be good enough, as I am
> > > still doing the same thing in setup(), i.e. setting time to just
> > > before y2038 to test if it is y2038 safe or not. I believe even that
> > > isn't fine ?
> >
> > Good point. I see you have this check above that:
> >
> > +       /* Check if the kernel is y2038 safe */
> > +       if (tv->type != TST_KERN_OLD_TIMESPEC)
> > +               return;
> > +
> >
> > So that part should prevent it from doing something bad,
>
> Not really. That code is part of the setup() routine and with "return"
> we will go and run the actual test run(). That code is there to avoid
> running the code on time64 implementation unnecessarily.

Ok, I see.

> > but I now
> > see it's still not great because it also prevents the test case from running
> > on 64-bit architectures. If you change the condition to also check for
> > sizeof(__kernel_old_timespec) it should be ok.
>
> What about this instead of the whole setup() changes:
>
>        /* Check if the kernel is y2038 safe */
>        if (tv->type == TST_KERN_OLD_TIMESPEC &&
>            sizeof(__kernel_old_timespec) == 8)
>                tst_brk("Invalid configuration");

Sounds good, as long as this only stops the clock_settime() run
and not all variants including the clock_settim64() one.

         Arnd

[LTP] [PATCH V3] syscalls: clock_settime: Add test around y2038 vulnerability

[Viresh Kumar]

This adds a test around the y2038 vulnerability, it sets the system time
to just before y2038 time (i.e. max value that can be stored in s32),
and adds a timer to expire just after crossing it.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
V3:
- Check for size of structure instead of running clock_settime().
- Check for 50 ms of delta in the time difference.

 runtest/syscalls                              |   1 +
 .../kernel/syscalls/clock_settime/.gitignore  |   1 +
 .../syscalls/clock_settime/clock_settime03.c  | 116 ++++++++++++++++++
 3 files changed, 118 insertions(+)
 create mode 100644 testcases/kernel/syscalls/clock_settime/clock_settime03.c

diff --git a/runtest/syscalls b/runtest/syscalls
index 8457db34e999..d7c3cbed611a 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -101,6 +101,7 @@ leapsec01 leapsec01
 
 clock_settime01 clock_settime01
 clock_settime02 clock_settime02
+clock_settime03 clock_settime03
 
 clone01 clone01
 clone02 clone02
diff --git a/testcases/kernel/syscalls/clock_settime/.gitignore b/testcases/kernel/syscalls/clock_settime/.gitignore
index 28121755006b..b66169b3eb7b 100644
--- a/testcases/kernel/syscalls/clock_settime/.gitignore
+++ b/testcases/kernel/syscalls/clock_settime/.gitignore
@@ -1,2 +1,3 @@
 clock_settime01
 clock_settime02
+clock_settime03
diff --git a/testcases/kernel/syscalls/clock_settime/clock_settime03.c b/testcases/kernel/syscalls/clock_settime/clock_settime03.c
new file mode 100644
index 000000000000..2d65564190b0
--- /dev/null
+++ b/testcases/kernel/syscalls/clock_settime/clock_settime03.c
@@ -0,0 +1,116 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2020 Linaro Limited. All rights reserved.
+ * Author: Viresh Kumar<viresh.kumar@linaro.org>
+ *
+ * Check Year 2038 related vulnerabilities.
+ */
+
+#include <signal.h>
+#include "config.h"
+#include "tst_timer.h"
+#include "tst_safe_clocks.h"
+
+#define TIMER_DELTA	3
+#define ALLOWED_DELTA	(50 * 1000) /* 50 ms */
+
+static struct tst_ts start, end;
+static struct tst_its its;
+
+static struct test_variants {
+	int (*clock_gettime)(clockid_t clk_id, void *ts);
+	int (*clock_settime)(clockid_t clk_id, void *ts);
+	int (*timer_settime)(timer_t timerid, int flags, void *its,
+			     void *old_its);
+	enum tst_ts_type type;
+	char *desc;
+} variants[] = {
+#if (__NR_clock_settime != __LTP__NR_INVALID_SYSCALL)
+	{ .clock_gettime = sys_clock_gettime, .clock_settime = sys_clock_settime, .timer_settime = sys_timer_settime, .type = TST_KERN_OLD_TIMESPEC, .desc = "syscall with old kernel spec"},
+#endif
+
+#if (__NR_clock_settime64 != __LTP__NR_INVALID_SYSCALL)
+	{ .clock_gettime = sys_clock_gettime64, .clock_settime = sys_clock_settime64, .timer_settime = sys_timer_settime64, .type = TST_KERN_TIMESPEC, .desc = "syscall time64 with kernel spec"},
+#endif
+};
+
+static void setup(void)
+{
+	struct test_variants *tv = &variants[tst_variant];
+
+	tst_res(TINFO, "Testing variant: %s", tv->desc);
+	start.type = end.type = its.type = tv->type;
+
+	/* Check if the kernel is y2038 safe */
+	if (tv->type != TST_KERN_OLD_TIMESPEC &&
+	    sizeof(start.ts.kern_old_ts) == 8)
+		tst_brk(TFAIL, "Not Y2038 safe to run test");
+}
+
+static void run(void)
+{
+	struct test_variants *tv = &variants[tst_variant];
+	unsigned long long time = 0x7FFFFFFE; /* Time just before y2038 */
+	struct sigevent ev = {
+		.sigev_notify = SIGEV_SIGNAL,
+		.sigev_signo = SIGABRT,
+	};
+	long long diff;
+	timer_t timer;
+	sigset_t set;
+	int sig, ret;
+
+	if (sigemptyset(&set) == -1)
+		tst_brk(TBROK, "sigemptyset() failed");
+
+	if (sigaddset(&set, SIGABRT) == -1)
+		tst_brk(TBROK, "sigaddset() failed");
+
+	if (sigprocmask(SIG_BLOCK, &set, NULL) == -1)
+		tst_brk(TBROK, "sigprocmask() failed");
+
+	TEST(tst_syscall(__NR_timer_create, CLOCK_REALTIME_ALARM, &ev, &timer));
+	if (TST_RET != 0)
+		tst_brk(TBROK | TERRNO, "timer_create() failed");
+
+	tst_ts_set_sec(&start, time);
+	tst_ts_set_nsec(&start, 0);
+
+	ret = tv->clock_settime(CLOCK_REALTIME, tst_ts_get(&start));
+	if (ret == -1)
+		tst_brk(TBROK | TERRNO, "clock_settime() failed");
+
+	tst_its_set_time(&its, time + TIMER_DELTA, 0, 0, 0);
+
+	TEST(tv->timer_settime(timer, TIMER_ABSTIME, tst_its_get(&its), NULL));
+	if (TST_RET == -1)
+		tst_brk(TBROK | TTERRNO, "timer_settime() failed");
+
+	if (sigwait(&set, &sig) == -1)
+		tst_brk(TBROK, "sigwait() failed");
+
+	ret = tv->clock_gettime(CLOCK_REALTIME, tst_ts_get(&end));
+	if (ret == -1)
+		tst_brk(TBROK | TERRNO, "clock_gettime() failed");
+
+	if (sig == SIGABRT) {
+		diff = tst_ts_diff_ms(end, start);
+
+		if (diff < TIMER_DELTA * 1000 - ALLOWED_DELTA ||
+		    diff > TIMER_DELTA * 1000 + ALLOWED_DELTA)
+			tst_res(TINFO, "Slept for unexpected duration, expected:%d, actual:%lld",
+				TIMER_DELTA * 1000, diff);
+		tst_res(TPASS, "clock_settime(): Y2038 test passed");
+		return;
+	}
+
+	tst_res(TFAIL, "clock_settime(): Y2038 test failed");
+}
+
+static struct tst_test test = {
+	.test_all = run,
+	.test_variants = ARRAY_SIZE(variants),
+	.setup = setup,
+	.needs_root = 1,
+	.restore_wallclock = 1,
+};
-- 
2.25.0.rc1.19.g042ed3e048af