All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] timekeeping: Make sure wall_to_monotonic isn't positive
@ 2021-12-13 13:57 Yu Liao
  2021-12-17 22:16 ` [tip: timers/urgent] timekeeping: Really make " tip-bot2 for Yu Liao
  0 siblings, 1 reply; 2+ messages in thread
From: Yu Liao @ 2021-12-13 13:57 UTC (permalink / raw)
  To: john.stultz; +Cc: tglx, linux-kernel, liwei391, liaoyu15

After commit e1d7ba873555 ("time: Always make sure wall_to_monotonic
isn't positive"), we can still change wall_to_monotonic to positive
by running the following code:
    int main(void)
    {
        struct timespec time;

        clock_gettime(CLOCK_MONOTONIC, &time);
        time.tv_nsec = 0;
        clock_settime(CLOCK_REALTIME, &time);
        return 0;
    }

The reason is that the parameters of timespec64_compare() may be
unnormalized, and timespec64_compare() simply compare tv_sec,
for example:
  wall_to_monotonic = {tv_sec = -10, tv_nsec = 900000000}
  ts_delta = {tv_sec = -9, tv_nsec = -900000000}

timespec64_compare() result is wall_to_monotonic < ts_delta, but
actually wall_to_monotonic > ts_delta. After normalized, the result
of timespec64_compare() is correct:
  wall_to_monotonic = {tv_sec = -10, tv_nsec = 900000000}
  ts_delta = {tv_sec = -10, tv_nsec = 100000000}

Use timespec64_sub() to ensure ts_delta normalized, and the code
looks more concise.

Signed-off-by: Yu Liao <liaoyu15@huawei.com>
---
 kernel/time/timekeeping.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index b348749a9fc6..dcdcb85121e4 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -1306,8 +1306,7 @@ int do_settimeofday64(const struct timespec64 *ts)
 	timekeeping_forward_now(tk);
 
 	xt = tk_xtime(tk);
-	ts_delta.tv_sec = ts->tv_sec - xt.tv_sec;
-	ts_delta.tv_nsec = ts->tv_nsec - xt.tv_nsec;
+	ts_delta = timespec64_sub(*ts, xt);
 
 	if (timespec64_compare(&tk->wall_to_monotonic, &ts_delta) > 0) {
 		ret = -EINVAL;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* [tip: timers/urgent] timekeeping: Really make sure wall_to_monotonic isn't positive
  2021-12-13 13:57 [PATCH v2] timekeeping: Make sure wall_to_monotonic isn't positive Yu Liao
@ 2021-12-17 22:16 ` tip-bot2 for Yu Liao
  0 siblings, 0 replies; 2+ messages in thread
From: tip-bot2 for Yu Liao @ 2021-12-17 22:16 UTC (permalink / raw)
  To: linux-tip-commits; +Cc: Yu Liao, Thomas Gleixner, stable, x86, linux-kernel

The following commit has been merged into the timers/urgent branch of tip:

Commit-ID:     4e8c11b6b3f0b6a283e898344f154641eda94266
Gitweb:        https://git.kernel.org/tip/4e8c11b6b3f0b6a283e898344f154641eda94266
Author:        Yu Liao <liaoyu15@huawei.com>
AuthorDate:    Mon, 13 Dec 2021 21:57:27 +08:00
Committer:     Thomas Gleixner <tglx@linutronix.de>
CommitterDate: Fri, 17 Dec 2021 23:06:22 +01:00

timekeeping: Really make sure wall_to_monotonic isn't positive

Even after commit e1d7ba873555 ("time: Always make sure wall_to_monotonic
isn't positive") it is still possible to make wall_to_monotonic positive
by running the following code:

    int main(void)
    {
        struct timespec time;

        clock_gettime(CLOCK_MONOTONIC, &time);
        time.tv_nsec = 0;
        clock_settime(CLOCK_REALTIME, &time);
        return 0;
    }

The reason is that the second parameter of timespec64_compare(), ts_delta,
may be unnormalized because the delta is calculated with an open coded
substraction which causes the comparison of tv_sec to yield the wrong
result:

  wall_to_monotonic = { .tv_sec = -10, .tv_nsec =  900000000 }
  ts_delta 	    = { .tv_sec =  -9, .tv_nsec = -900000000 }

That makes timespec64_compare() claim that wall_to_monotonic < ts_delta,
but actually the result should be wall_to_monotonic > ts_delta.

After normalization, the result of timespec64_compare() is correct because
the tv_sec comparison is not longer misleading:

  wall_to_monotonic = { .tv_sec = -10, .tv_nsec =  900000000 }
  ts_delta 	    = { .tv_sec = -10, .tv_nsec =  100000000 }

Use timespec64_sub() to ensure that ts_delta is normalized, which fixes the
issue.

Fixes: e1d7ba873555 ("time: Always make sure wall_to_monotonic isn't positive")
Signed-off-by: Yu Liao <liaoyu15@huawei.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: stable@vger.kernel.org
Link: https://lore.kernel.org/r/20211213135727.1656662-1-liaoyu15@huawei.com
---
 kernel/time/timekeeping.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index b348749..dcdcb85 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -1306,8 +1306,7 @@ int do_settimeofday64(const struct timespec64 *ts)
 	timekeeping_forward_now(tk);
 
 	xt = tk_xtime(tk);
-	ts_delta.tv_sec = ts->tv_sec - xt.tv_sec;
-	ts_delta.tv_nsec = ts->tv_nsec - xt.tv_nsec;
+	ts_delta = timespec64_sub(*ts, xt);
 
 	if (timespec64_compare(&tk->wall_to_monotonic, &ts_delta) > 0) {
 		ret = -EINVAL;

^ permalink raw reply related	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2021-12-17 22:16 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-13 13:57 [PATCH v2] timekeeping: Make sure wall_to_monotonic isn't positive Yu Liao
2021-12-17 22:16 ` [tip: timers/urgent] timekeeping: Really make " tip-bot2 for Yu Liao

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.