From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755662AbbBLN6w (ORCPT ); Thu, 12 Feb 2015 08:58:52 -0500 Received: from mx1.redhat.com ([209.132.183.28]:39803 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755282AbbBLN6v (ORCPT ); Thu, 12 Feb 2015 08:58:51 -0500 From: Prarit Bhargava To: linux-kernel@vger.kernel.org Cc: Prarit Bhargava , John Stultz , Thomas Gleixner , Miroslav Lichvar , Peter Zijlstra Subject: [PATCH] time, ntp: Do not update time_state in middle of leap second [v3] Date: Thu, 12 Feb 2015 08:58:19 -0500 Message-Id: <1423749499-18520-1-git-send-email-prarit@redhat.com> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org During leap second insertion testing it was noticed that a small window exists where the time_state could be reset such that time_state = TIME_OK, which then causes the leap second to not occur, or causes the entire leap second state machine to fail with time_state = TIME_INS at the end of the leap second. The test did the following in userspace: tx.modes = ADJ_STATUS; tx.status = STA_INS; /* send leap second request */ ret = adjtimex(&tx); /* Check adjtimex output every half second */ now = tx.time.tv_sec; while (now < next_leap+2) { char buf[26]; ret = adjtimex(&tx); ctime_r(&tx.time.tv_sec, buf); buf[strlen(buf)-1] = 0; /*remove trailing\n */ printf("%s + %6ld us\t%s\n", buf, tx.time.tv_usec, time_state_str(ret)); now = tx.time.tv_sec; /* Sleep for another half second */ ts.tv_sec = 0; ts.tv_nsec = NSEC_PER_SEC/2; clock_nanosleep(CLOCK_MONOTONIC, 0, &ts, NULL); } which was intended to mimic the insertion of a leap second. A successful run of the test would result in the time_state transitioning from TIME_OK to TIME_INS, then to TIME_OOP when the leap second was inserted, and then to TIME_WAIT when the leap second was completed. While running this code failures were seen in which the time_state remained TIME_INS, even though the leap second had occurred. After some investigation it was noted that the test contained a small error: the test does not reinitialize tx.status and reissues the STA_INS every 1/2 second. As a result of this broken test, the following failure was noticed (the output below is a mix of kernel messages and the output from the test program, the remaining annotations are printk's in the code and my own additional notes): [ 942.952833] time_state [1] change from TIME_OK to TIME_INS Fri Feb 13 18:59:51 2015 + 318126 us TIME_INS Fri Feb 13 18:59:51 2015 + 818167 us TIME_INS Fri Feb 13 18:59:52 2015 + 318208 us TIME_INS Fri Feb 13 18:59:52 2015 + 818248 us TIME_INS Fri Feb 13 18:59:53 2015 + 318290 us TIME_INS Fri Feb 13 18:59:53 2015 + 818331 us TIME_INS Fri Feb 13 18:59:54 2015 + 318372 us TIME_INS Fri Feb 13 18:59:54 2015 + 818413 us TIME_INS Fri Feb 13 18:59:55 2015 + 318454 us TIME_INS Fri Feb 13 18:59:55 2015 + 818495 us TIME_INS Fri Feb 13 18:59:56 2015 + 318534 us TIME_INS Fri Feb 13 18:59:56 2015 + 818575 us TIME_INS Fri Feb 13 18:59:57 2015 + 318617 us TIME_INS Fri Feb 13 18:59:57 2015 + 818660 us TIME_INS Fri Feb 13 18:59:58 2015 + 318702 us TIME_INS Fri Feb 13 18:59:58 2015 + 818744 us TIME_INS Fri Feb 13 18:59:59 2015 + 318785 us TIME_INS Fri Feb 13 18:59:59 2015 + 818837 us TIME_INS [ 952.953143] time_state [4] change from TIME_INS to TIME_OOP [ 952.953150] Clock: inserting leap second 23:59:60 UTC [ 953.299905] process_adj_status: insert_leap_sec[1223] setting time_state back to TIME_OK [1, 1] <<< adjtimex() call every 1/2 second [ 953.299913] time_state [9] change from TIME_OOP to TIME_OK Fri Feb 13 18:59:59 2015 + 318878 us TIME_OK Fri Feb 13 18:59:59 2015 + 818931 us TIME_OK [ 954.064237] time_state [1] change from TIME_OK to TIME_INS Fri Feb 13 19:00:00 2015 + 318972 us TIME_INS Fri Feb 13 19:00:00 2015 + 819012 us TIME_INS Fri Feb 13 19:00:01 2015 + 319051 us TIME_INS Fri Feb 13 19:00:01 2015 + 819089 us TIME_INS Fri Feb 13 19:00:02 2015 + 319128 us TIME_INS As previously stated, the time_state remains TIME_INS even though the leap second has already occurred @ 952.953150. The test was changed to reset tx.status to 0 in the loop, and the test then succeeded with a 100% rate with the time state ending in TIME_WAIT. While this is highly unlikely to ever happen in the real world it is still something we should protect against, as breaking the state machine is bad. If the time_state == TIME_OOP (ie, the leap second is in progress) do not allow an external update to time_state in process_adj_status(). This will prevent external adjtimex() calls from breaking the leap second state machine. [v2]: Only block time_state change when TIME_OOP [v3]: Write a much more detailed explanation of the bug. Signed-off-by: Prarit Bhargava Cc: John Stultz Cc: Thomas Gleixner Cc: Miroslav Lichvar Cc: Peter Zijlstra --- kernel/time/ntp.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/kernel/time/ntp.c b/kernel/time/ntp.c index 28bf91c..6ff5cd5 100644 --- a/kernel/time/ntp.c +++ b/kernel/time/ntp.c @@ -535,7 +535,8 @@ void ntp_notify_cmos_timer(void) { } static inline void process_adj_status(struct timex *txc, struct timespec64 *ts) { if ((time_status & STA_PLL) && !(txc->status & STA_PLL)) { - time_state = TIME_OK; + if (time_state != TIME_OOP) + time_state = TIME_OK; time_status = STA_UNSYNC; /* restart PPS frequency calibration */ pps_reset_freq_interval(); -- 1.7.9.3