2021-12-13 13:51:45

by Yu Liao

[permalink] [raw]
Subject: [PATCH v2] timekeeping: Make sure wall_to_monotonic isn't positive

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 <[email protected]>
---
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



2021-12-17 22:16:05

by tip-bot2 for Jacob Pan

[permalink] [raw]
Subject: [tip: timers/urgent] timekeeping: Really make sure wall_to_monotonic isn't positive

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 <[email protected]>
AuthorDate: Mon, 13 Dec 2021 21:57:27 +08:00
Committer: Thomas Gleixner <[email protected]>
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 <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
Cc: [email protected]
Link: https://lore.kernel.org/r/[email protected]
---
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;