From: Dave Hart <hart@ntp.org>
Date: Wed, 22 Feb 2012 05:05:04 +0000 (+0000)
Subject: [Bug 2148] ntpd 4.2.7p258 segfault with 0x0100000 bit in NMEA mode.
X-Git-Tag: NTP_4_2_7P259~1
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=f4611fc5704583c7b71ee7aa90f0293bbc699bf1;p=thirdparty%2Fntp.git

[Bug 2148] ntpd 4.2.7p258 segfault with 0x0100000 bit in NMEA mode.
refclock_nmea.c merge cleanup thanks to Juergen Perlinger.
Use mutex with Lamport checks in get_systime() on Windows, which
  unlike portable ntpd uses the routine across threads.

bk: 4f447780WcagGVkhY7Nx3Nz2XtBGWg
---

diff --git a/ChangeLog b/ChangeLog
index cb078f78d..7bebf88c1 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,5 @@
+* [Bug 2148] ntpd 4.2.7p258 segfault with 0x0100000 bit in NMEA mode.
+* refclock_nmea.c merge cleanup thanks to Juergen Perlinger.
 (4.2.7p258) 2012/02/21 Released by Harlan Stenn <stenn@ntp.org>
 * [Bug 2140] Rework of Windows I/O completion port handling to avoid
   garbling serial input in UNIX line discipline emulation.
diff --git a/include/ntp_assert.h b/include/ntp_assert.h
index ab9b41981..206d50d26 100644
--- a/include/ntp_assert.h
+++ b/include/ntp_assert.h
@@ -88,10 +88,10 @@ extern void calysto_assert(unsigned char cnd); /* check whether this holds */
 #define	DEBUG_INVARIANT(x)	INVARIANT(x)
 #define	DEBUG_ENSURE(x)		ENSURE(x)
 # else
-#define	DEBUG_REQUIRE(x)	(void)(x)
-#define	DEBUG_INSIST(x)		(void)(x)
-#define	DEBUG_INVARIANT(x)	(void)(x)
-#define	DEBUG_ENSURE(x)		(void)(x)
+#define	DEBUG_REQUIRE(x)	do {} while (FALSE)
+#define	DEBUG_INSIST(x)		do {} while (FALSE)
+#define	DEBUG_INVARIANT(x)	do {} while (FALSE)
+#define	DEBUG_ENSURE(x)		do {} while (FALSE)
 # endif
 
 #endif	/* NTP_ASSERT_H */
diff --git a/include/ntp_fp.h b/include/ntp_fp.h
index 1eeca3747..bb58bf1f8 100644
--- a/include/ntp_fp.h
+++ b/include/ntp_fp.h
@@ -357,6 +357,7 @@ extern	char *	uglydate	(l_fp *);
 extern  void	mfp_mul		(int32 *, u_int32 *, int32, u_int32, int32, u_int32);
 
 extern	void	set_sys_fuzz	(double);
+extern	void	init_systime	(void);
 extern	void	get_systime	(l_fp *);
 extern	int	step_systime	(double);
 extern	int	adj_systime	(double);
@@ -383,4 +384,33 @@ extern	struct tm * ntp2unix_tm (u_int32 ntp, int local);
  */
 typedef void (*time_stepped_callback)(void);
 extern time_stepped_callback	step_callback;
+
+/*
+ * Multi-thread locking for get_systime()
+ *
+ * On most systems, get_systime() is used solely by the main ntpd
+ * thread, but on Windows it's also used by the dedicated I/O thread.
+ * The [Bug 2037] changes to get_systime() have it keep state between
+ * calls to ensure time moves in only one direction, which means its
+ * use on Windows needs to be protected against simultaneous execution
+ * to avoid falsely detecting Lamport violations by ensuring only one
+ * thread at a time is in get_systime().
+ */
+#ifdef SYS_WINNT
+extern CRITICAL_SECTION get_systime_cs;
+# define INIT_GET_SYSTIME_CRITSEC()				\
+		InitializeCriticalSection(&get_systime_cs)
+# define ENTER_GET_SYSTIME_CRITSEC()				\
+		EnterCriticalSection(&get_systime_cs)
+# define LEAVE_GET_SYSTIME_CRITSEC()				\
+		LeaveCriticalSection(&get_systime_cs)
+#else	/* !SYS_WINNT follows */
+# define INIT_GET_SYSTIME_CRITSEC()			\
+		do {} while (FALSE)
+# define ENTER_GET_SYSTIME_CRITSEC()			\
+		do {} while (FALSE)
+# define LEAVE_GET_SYSTIME_CRITSEC()			\
+		do {} while (FALSE)
+#endif
+
 #endif /* NTP_FP_H */
diff --git a/libntp/lib_strbuf.c b/libntp/lib_strbuf.c
index 9ddf5cf50..76f70163d 100644
--- a/libntp/lib_strbuf.c
+++ b/libntp/lib_strbuf.c
@@ -7,9 +7,12 @@
 
 #include <isc/net.h>
 #include <isc/result.h>
+
+#include "ntp_fp.h"
 #include "ntp_stdlib.h"
 #include "lib_strbuf.h"
 
+
 /*
  * Storage declarations
  */
@@ -20,6 +23,7 @@ int		ipv4_works;
 int		ipv6_works;
 int		lib_inited;
 
+
 /*
  * initialization routine.  Might be needed if the code is ROMized.
  */
@@ -30,5 +34,6 @@ init_lib(void)
 		return;
 	ipv4_works = (ISC_R_SUCCESS == isc_net_probeipv4());
 	ipv6_works = (ISC_R_SUCCESS == isc_net_probeipv6());
+	init_systime();
 	lib_inited = TRUE;
 }
diff --git a/libntp/systime.c b/libntp/systime.c
index 70a4af5fc..fbe403b98 100644
--- a/libntp/systime.c
+++ b/libntp/systime.c
@@ -76,7 +76,18 @@ time_stepped_callback	step_callback;
 
 #ifndef SIM
 static int lamport_violated;	/* clock was stepped back */
+#endif	/* !SIM */
+
+#ifdef DEBUG
+static int systime_init_done;
+# define DONE_SYSTIME_INIT()	systime_init_done = TRUE
+#else
+# define DONE_SYSTIME_INIT()	do {} while (FALSE)
 #endif
+#ifdef SYS_WINNT
+CRITICAL_SECTION get_systime_cs;
+#endif
+
 
 void
 set_sys_fuzz(
@@ -90,6 +101,14 @@ set_sys_fuzz(
 }
 
 
+void
+init_systime(void)
+{
+	INIT_GET_SYSTIME_CRITSEC();
+	DONE_SYSTIME_INIT();
+}
+
+
 #ifndef SIM	/* ntpsim.c has get_systime() and friends for sim */
 
 static inline void
@@ -134,6 +153,7 @@ get_systime(
 {
 	static struct timespec	ts_prev;	/* prior os time */
 	static l_fp		lfp_prev;	/* prior result */
+	static double		dfuzz_prev;	/* prior fuzz */
 	struct timespec ts;	/* seconds and nanoseconds */
 	struct timespec ts_min;	/* earliest permissible */
 	struct timespec ts_lam;	/* lamport fictional increment */
@@ -145,6 +165,8 @@ get_systime(
 	l_fp	lfpdelta;
 
 	get_ostime(&ts);
+	DEBUG_REQUIRE(systime_init_done);
+	ENTER_GET_SYSTIME_CRITSEC();
 
 	/*
 	 * After default_get_precision() has set a nonzero sys_fuzz,
@@ -188,8 +210,8 @@ get_systime(
 			msyslog(LOG_ERR, "ts_min %s ts_prev %s ts %s",
 				tspectoa(ts_min), tspectoa(ts_prev_log),
 				tspectoa(ts));
-			msyslog(LOG_ERR, "sys_fuzz %ld nsec, this fuzz %.9f",
-				sys_fuzz_nsec, dfuzz);
+			msyslog(LOG_ERR, "sys_fuzz %ld nsec, this fuzz %.9f, prior %.9f",
+				sys_fuzz_nsec, dfuzz, dfuzz_prev);
 			lfpdelta = lfp_prev;
 			L_SUB(&lfpdelta, &result);
 			LFPTOD(&lfpdelta, ddelta);
@@ -200,9 +222,11 @@ get_systime(
 		}
 	}
 	lfp_prev = result;
+	dfuzz_prev = dfuzz;
 	if (lamport_violated) 
 		lamport_violated = FALSE;
 
+	LEAVE_GET_SYSTIME_CRITSEC();
 	*now = result;
 }
 
diff --git a/ntpd/ntp_control.c b/ntpd/ntp_control.c
index 17456b110..7f6d116ab 100644
--- a/ntpd/ntp_control.c
+++ b/ntpd/ntp_control.c
@@ -2454,7 +2454,13 @@ ctl_putpeer(
 		break;
 
 	case CP_TTL:
-		if (p->ttl)
+#ifdef REFCLOCK
+		if (p->flags & FLAG_REFCLOCK) {
+			ctl_putuint(peer_var[id].text, p->ttl);
+			break;
+		}
+#endif
+		if (p->ttl > 0 && p->ttl < COUNTOF(sys_ttl))
 			ctl_putint(peer_var[id].text,
 				   sys_ttl[p->ttl]);
 		break;
diff --git a/ntpd/ntp_refclock.c b/ntpd/ntp_refclock.c
index 22ca7ff04..65097150a 100644
--- a/ntpd/ntp_refclock.c
+++ b/ntpd/ntp_refclock.c
@@ -609,7 +609,9 @@ refclock_gtlin(
 	*tsptr = rbufp->recv_time;
 	DPRINTF(2, ("refclock_gtlin: fd %d time %s timecode %d %s\n",
 		    rbufp->fd, ulfptoa(&rbufp->recv_time, 6), dlen,
-		    lineptr));
+		    (dlen != 0)
+			? lineptr
+			: ""));
 	return (dlen);
 }
 
diff --git a/ntpd/ntpd.c b/ntpd/ntpd.c
index 6073f32df..589a0c6fc 100644
--- a/ntpd/ntpd.c
+++ b/ntpd/ntpd.c
@@ -558,6 +558,7 @@ ntpdmain(
 	} while (0);				/* 'loop' once */
 # endif	/* HAVE_WORKING_FORK */
 
+	init_lib();
 # ifdef SYS_WINNT
 	/*
 	 * Start interpolation thread, must occur before first
@@ -763,7 +764,6 @@ ntpdmain(
 	init_restrict();
 	init_mon();
 	init_timer();
-	init_lib();
 	init_request();
 	init_control();
 	init_peer();
diff --git a/ntpd/refclock_nmea.c b/ntpd/refclock_nmea.c
index 88875a209..13bcca2b9 100644
--- a/ntpd/refclock_nmea.c
+++ b/ntpd/refclock_nmea.c
@@ -241,11 +241,6 @@ typedef struct {
 		tally;
 	/* per sentence checksum seen flag */
 	u_char	cksum_type[NMEA_ARRAY_SIZE];
-	int	total_nmea;	/* clockstats sentence counts */
-	int	good_nmea;
-	int	bad_nmea;
-	int	filter_nmea;
-	int	pps_used;
 } nmea_unit;
 
 /*
@@ -995,14 +990,6 @@ nmea_receive(
 	pp->a_lastcode[rd_lencode] = '\0';
 	pp->lastrec = rd_timestamp;
 
-	if (pp->leap == LEAP_NOTINSYNC) {
-		up->bad_nmea++;
-		return;
-	}
-
-	/* Text looks OK. */
-	up->good_nmea++;
-
 #ifdef HAVE_PPSAPI
 	/*
 	 * If we have PPS running, we try to associate the sentence
diff --git a/ports/winnt/ntpd/ntp_iocompletionport.c b/ports/winnt/ntpd/ntp_iocompletionport.c
index cbec92e36..9e3e541f3 100644
--- a/ports/winnt/ntpd/ntp_iocompletionport.c
+++ b/ports/winnt/ntpd/ntp_iocompletionport.c
@@ -615,7 +615,7 @@ OnSerialWaitComplete(
 			lpo->DCDSTime = lpo->RecvTime;
 			lpo->flTsDCDS = 1;
 			DPRINTF(2, ("fd %d DCD PPS at %s\n", rio->fd,
-				    lfptoa(&lpo->RecvTime, 6)));
+				    ulfptoa(&lpo->RecvTime, 6)));
 		}
 	}
 
diff --git a/sntp/tests_main.cpp b/sntp/tests_main.cpp
index beb51cab6..d9d6277b1 100644
--- a/sntp/tests_main.cpp
+++ b/sntp/tests_main.cpp
@@ -3,6 +3,8 @@
 int main(int argc, char **argv) {
 	::testing::InitGoogleTest(&argc, argv);
 
+	init_lib();
+
 	// Some tests makes use of extra parameters passed to the tests
 	// executable. Save these params as static members of the base class.
 	if (argc > 1) {
diff --git a/sntp/tests_main.h b/sntp/tests_main.h
index d280da80a..991f4b6d0 100644
--- a/sntp/tests_main.h
+++ b/sntp/tests_main.h
@@ -8,6 +8,9 @@
 
 #include <gtest/gtest.h>
 
+extern "C" {
+#include "ntp_stdlib.h"
+}
 
 class ntptest : public ::testing::Test {
 public: