Skip to content

Commit 87d571d

Browse files
JustinStittKAGA-KOKO
authored andcommitted
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 eea83d8 ("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: eea83d8 ("ntp: NTP4 user space bits update") Signed-off-by: Justin Stitt <[email protected]> Signed-off-by: Thomas Gleixner <[email protected]> Cc: Miroslav Lichvar <[email protected]> Link: https://lore.kernel.org/all/[email protected] Closes: KSPP/linux#354
1 parent de9c2c6 commit 87d571d

File tree

1 file changed

+2
-2
lines changed

1 file changed

+2
-2
lines changed

kernel/time/ntp.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -727,10 +727,10 @@ static inline void process_adjtimex_modes(const struct __kernel_timex *txc,
727727
}
728728

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

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

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

0 commit comments

Comments
 (0)