linux-kernel - [tip: timers/urgent] ntp: Clamp maxerror and esterror to operating range

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <172286772988.2215.12627503289545685175.tip-bot2@tip-bot2>
Date: Mon, 05 Aug 2024 14:22:09 -0000
From: "tip-bot2 for Justin Stitt" <tip-bot2@...utronix.de>
To: linux-tip-commits@...r.kernel.org
Cc: Justin Stitt <justinstitt@...gle.com>,
 Thomas Gleixner <tglx@...utronix.de>, Miroslav Lichvar <mlichvar@...hat.com>,
 x86@...nel.org, linux-kernel@...r.kernel.org
Subject:
 [tip: timers/urgent] ntp: Clamp maxerror and esterror to operating range

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

Commit-ID:     87d571d6fb77ec342a985afa8744bb9bb75b3622
Gitweb:        https://git.kernel.org/tip/87d571d6fb77ec342a985afa8744bb9bb75b3622
Author:        Justin Stitt <justinstitt@...gle.com>
AuthorDate:    Fri, 17 May 2024 20:22:44 
Committer:     Thomas Gleixner <tglx@...utronix.de>
CommitterDate: Mon, 05 Aug 2024 16:14:14 +02:00

ntp: Clamp maxerror and esterror to operating range

Using syzkaller alongside the newly reintroduced signed integer overflow
sanitizer spits out this report:

UBSAN: signed-integer-overflow in ../kernel/time/ntp.c:461:16
9223372036854775807 + 500 cannot be represented in type 'long'
Call Trace:
 handle_overflow+0x171/0x1b0
 second_overflow+0x2d6/0x500
 accumulate_nsecs_to_secs+0x60/0x160
 timekeeping_advance+0x1fe/0x890
 update_wall_time+0x10/0x30

time_maxerror is unconditionally incremented and the result is checked
against NTP_PHASE_LIMIT, but the increment itself can overflow, resulting
in wrap-around to negative space.

Before commit eea83d896e31 ("ntp: NTP4 user space bits update") the user
supplied value was sanity checked to be in the operating range. That change
removed the sanity check and relied on clamping in handle_overflow() which
does not work correctly when the user supplied value is in the overflow
zone of the '+ 500' operation.

The operation requires CAP_SYS_TIME and the side effect of the overflow is
NTP getting out of sync.

Miroslav confirmed that the input value should be clamped to the operating
range and the same applies to time_esterror. The latter is not used by the
kernel, but the value still should be in the operating range as it was
before the sanity check got removed.

Clamp them to the operating range.

[ tglx: Changed it to clamping and included time_esterror ] 

Fixes: eea83d896e31 ("ntp: NTP4 user space bits update")
Signed-off-by: Justin Stitt <justinstitt@...gle.com>
Signed-off-by: Thomas Gleixner <tglx@...utronix.de>
Cc: Miroslav Lichvar <mlichvar@...hat.com>
Link: https://lore.kernel.org/all/20240517-b4-sio-ntp-usec-v2-1-d539180f2b79@google.com
Closes: https://github.com/KSPP/linux/issues/354
---
 kernel/time/ntp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/time/ntp.c b/kernel/time/ntp.c
index 406dccb..502e1e5 100644
--- a/kernel/time/ntp.c
+++ b/kernel/time/ntp.c
@@ -727,10 +727,10 @@ static inline void process_adjtimex_modes(const struct __kernel_timex *txc,
 	}

 	if (txc->modes & ADJ_MAXERROR)
-		time_maxerror = txc->maxerror;
+		time_maxerror = clamp(txc->maxerror, 0, NTP_PHASE_LIMIT);

 	if (txc->modes & ADJ_ESTERROR)
-		time_esterror = txc->esterror;
+		time_esterror = clamp(txc->esterror, 0, NTP_PHASE_LIMIT);

 	if (txc->modes & ADJ_TIMECONST) {
 		time_constant = txc->constant;