timekeeping: always make sure wall_to_monotonic isn't positive

I meet two issues on an IMX6 development board without enable RTC device(so timekeeping_init will initialize the boot time and monotonic to 0).

Issue 1:exportfs -a generate:
"exportfs: /opt/nfs/arm does not support NFS export"
Issue 2:cat /proc/stat:

       "btime 4294967236"

Exact reproduction of the same issues on x86 after run below code:
" int main(void)

        {
            struct timeval val;
            int ret;

            val.tv_sec = 0;
            val.tv_usec = 0;
            ret = settimeofday(&val, NULL);
            return 0;
        }

"
Reason:
The reason is positive wall_to_monotonic push boot time back to the time before Epoch, getboottime will return negative value.

In issue 1:
negative boot time cause get_expiry overflow time_t when input expire time is 2147483647, then cache_flush always clear entries just added in ip_map_parse.
In issue 2:
show_stat use "unsigned long" to print
negative value return by getboottime.

This patch fix these two issues.

Note: this patch will cause we can't use settimeofday with time
earlier than current time on system which timekeeping_init initialize the xtime, boot and monotonic to 0 before set current time to a more reasonable time point.

Signed-off-by Wang YanQing <[email protected]>

---
Changes v1-v2:
1: fix subject, use "isn't positive" instead of "is negative". 2: rewrite changelog.
3: simplify code as suggested by John Stultz.

It really take me some times to realize how stupid and buggy the version 1 patch is, but I am ready to be told this version is even stupider:)

Thanks.

 kernel/time/timekeeping.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

kernel/time/timekeeping.c +2 -0

index 0d784b3..b501aa6 100644
@@ -895,6 +895,7 @@ int do_settimeofday64(const struct timespec64 *ts)
        struct timekeeper *tk = &tk_core.timekeeper;
        struct timespec64 ts_delta, xt;
        unsigned long flags;
+       int ret = 0;
 
        if (!timespec64_valid_strict(ts))
                return -EINVAL;

@@ -908,10 +909,15 @@ int do_settimeofday64(const struct timespec64 *ts)
        ts_delta.tv_sec = ts->tv_sec - xt.tv_sec;
        ts_delta.tv_nsec = ts->tv_nsec - xt.tv_nsec;
 
+       if (timespec64_compare(&tk->wall_to_monotonic, &ts_delta) > 0) {

kernel/time/timekeeping.c +1 -1

        write_seqcount_end(&tk_core.seq);

@@ -920,7 +926,7 @@ int do_settimeofday64(const struct timespec64 *ts)
        /* signal hrtimers about time change */
        clock_was_set();
 
-       return 0;
+       return ret;
 }
 EXPORT_SYMBOL(do_settimeofday64);
 

Quoted 34 lines · Wang YanQing

I meet two issues on an IMX6 development board without enable RTC device(so timekeeping_init will initialize the boot time and monotonic to 0).

Issue 1:exportfs -a generate:
"exportfs: /opt/nfs/arm does not support NFS export"
Issue 2:cat /proc/stat:

       "btime 4294967236"

Exact reproduction of the same issues on x86 after run below code:
" int main(void)

        {
            struct timeval val;
            int ret;

            val.tv_sec = 0;
            val.tv_usec = 0;
            ret = settimeofday(&val, NULL);
            return 0;
        }

"
Reason:
The reason is positive wall_to_monotonic push boot time back to the time before Epoch, getboottime will return negative value.

In issue 1:
negative boot time cause get_expiry overflow time_t when input expire time is 2147483647, then cache_flush always clear entries just added in ip_map_parse.
In issue 2:
show_stat use "unsigned long" to print
negative value return by getboottime.

Quoted 1 line

This patch fix these two issues.

If there is two issues, we probably should have two patches, each clearly fixing one issue. If there is one problem with multiple symptoms, then a single patch is fine but we want to be clear there.

Quoted 4 lines

Note: this patch will cause we can't use settimeofday with time
earlier than current time on system which timekeeping_init initialize the xtime, boot and monotonic to 0 before set current time to a more reasonable time point.

If everything is initialized to 0 (aka 1970), then setting the time to prior to (relatively) shortly after boot is a pretty reasonable constraint. So you might want to reword this a little bit.

This basically seems to come down to the fact that you can't set the CLOCK_REALTIME time prior to (1970 + system uptime), right?

Quoted 42 lines

Signed-off-by Wang YanQing <[email protected]>

---
Changes v1-v2:
1: fix subject, use "isn't positive" instead of "is negative". 2: rewrite changelog.
3: simplify code as suggested by John Stultz.

It really take me some times to realize how stupid and buggy the version 1 patch is, but I am ready to be told this version is even stupider:)

Thanks.

 kernel/time/timekeeping.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

kernel/time/timekeeping.c +6 -0

index 0d784b3..b501aa6 100644
@@ -895,6 +895,7 @@ int do_settimeofday64(const struct timespec64 *ts)
        struct timekeeper *tk = &tk_core.timekeeper;
        struct timespec64 ts_delta, xt;
        unsigned long flags;
+       int ret = 0;
 
        if (!timespec64_valid_strict(ts))
                return -EINVAL;

@@ -908,10 +909,15 @@ int do_settimeofday64(const struct timespec64 *ts)
        ts_delta.tv_sec = ts->tv_sec - xt.tv_sec;
        ts_delta.tv_nsec = ts->tv_nsec - xt.tv_nsec;
 
+       if (timespec64_compare(&tk->wall_to_monotonic, &ts_delta) > 0) {
+               ret = -EINVAL;
+               goto out;
+       }
+
        tk_set_wall_to_mono(tk, timespec64_sub(tk->wall_to_monotonic, ts_delta));
 

kernel/time/timekeeping.c int do_settimeofday64(const struct timespec64 *ts) · line 921

Quoted 4 lines

kernel/time/timekeeping.c +1 -1

        tk_set_xtime(tk, ts);
-
+out:
        timekeeping_update(tk, TK_CLEAR_NTP | TK_MIRROR | TK_CLOCK_WAS_SET);

If we didn't set the time, should we be calling timekeeping_update here?