From: Juergen Perlinger Date: Fri, 2 Jan 2015 12:45:05 +0000 (+0100) Subject: [Bug 2627] shm refclock allows only two units with owner-only access X-Git-Tag: NTP_4_2_8P1_BETA3~1^2~2^2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=24553b61fa8e88f3fd4afeb8f905b7ff78ae7fc1;p=thirdparty%2Fntp.git [Bug 2627] shm refclock allows only two units with owner-only access bk: 54a692d1Q3urnDhQDFKyn6ZmYi-Ajw --- diff --git a/ChangeLog b/ChangeLog index 10b5d225e..abd197f87 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,6 @@ +* [Bug 2627] shm refclock allows only two units with owner-only access + Use mode bit 0 to select public access for units >= 2 (units 0 & 1 are + always private --- * [Sec 2672] On some OSes ::1 can be spoofed, bypassing source IP ACLs. diff --git a/html/drivers/driver28.html b/html/drivers/driver28.html index 8c7fd802e..df1a588fd 100644 --- a/html/drivers/driver28.html +++ b/html/drivers/driver28.html @@ -7,6 +7,10 @@ Shared Memory Driver + @@ -21,7 +25,13 @@ Driver ID: SHM

Description

-

This driver receives its reference clock info from a shared memory-segment. The shared memory-segment is created with owner-only access for unit 0 and 1, and world access for unit 2 and 3

+

This driver receives its reference clock info from a shared + memory-segment. The shared memory-segment is created with owner-only + access by default, unless otherwise requested by the mode word for units + ≥2. Units 0 and 1 are always created with owner-only access for + backward compatibility. +

+

Structure of shared memory-segment

struct shmTime {
@@ -49,14 +59,14 @@
 
         
Operation mode=0

         
Each second, the value of valid of the shared memory-segment is checked:

-        
If set, the values in the record (clockTimeStampSec, clockTimeStampUSec, receiveTimeStampSec, receiveTimeStampUSec, leap, precision) are passed to ntp, and valid is cleared and count is bumped.

+        
If set, the values in the record (clockTimeStampSec, clockTimeStampUSec, receiveTimeStampSec, receiveTimeStampUSec, leap, precision) are passed to NTPD, and valid is cleared and count is bumped.

         
If not set, count is bumped.

         
Operation mode=1

         
Each second, valid in the shared memory-segment is checked:

-        
If set, the count field of the record is remembered, and the values in the record (clockTimeStampSec, clockTimeStampUSec, receiveTimeStampSec, receiveTimeStampUSec, leap, precision) are read. Then, the remembered count is compared to current value of count now in the record. If both are equal, the values read from the record are passed to ntp. If they differ, another process has modified the record while it was read out (was not able to produce this case), and failure is reported to ntp. The valid flag is cleared and count is bumped.

+        
If set, the count field of the record is remembered, and the values in the record (clockTimeStampSec, clockTimeStampUSec, receiveTimeStampSec, receiveTimeStampUSec, leap, precision) are read. Then, the remembered count is compared to current value of count now in the record. If both are equal, the values read from the record are passed to NTPD. If they differ, another process has modified the record while it was read out (was not able to produce this case), and failure is reported to NTPD. The valid flag is cleared and count is bumped.

         
If not set, count is bumped

 
-
Mode-independent postprocessing

+
Mode-independent post-processing

 After the time stamps have been successfully plucked from the SHM
 segment, some sanity checks take place:
 
 
-
gpsd

+
GPSD

 
-gpsd
+GPSD
 knows how to talk to many GPS devices.
-It can work with ntpd through the SHM driver.
+It can work with NTPD through the SHM driver.
 

-The gpsd man page suggests setting minpoll and maxpoll to 4.
+The GPSD man page suggests setting minpoll and maxpoll to 4.
 That was an attempt to reduce jitter.
 The SHM driver was fixed (ntp-4.2.5p138) to collect data each second rather than
 once per polling interval so that suggestion is no longer reasonable.
 

-  Note: The GPSD client driver (type 46) uses the gpsd
-  client protocol to connect and talk to gpsd, but using the
-  SHM driver is the ancient way to have gpsd talk to ntpd.
+  Note: The GPSD client driver (type 46) uses the GPSD
+  client protocol to connect and talk to GPSD, but using the
+  SHM driver is the ancient way to have GPSD talk to NTPD. There
+  are some tricky points when using the SHM interface to interface
+  with GPSD, because GPSD will use two SHM clocks, one for the
+  serial data stream and one for the PPS information when
+  available. Receivers with a loose/sloppy timing between PPS and serial data
+  can easily cause trouble here because NTPD has no way to join the two
+  data streams and correlate the serial data with the PPS events.
+

+

 
 
Clockstats

 If flag4 is set when the driver is polled, a clockstats record is written.
 The first 3 fields are the normal date, time, and IP address common to all clockstats records.
 

 The 4th field is the number of second ticks since the last poll.
-The 5th field is the number of good data samples found.  The last 64 will be used by ntpd.
+The 5th field is the number of good data samples found.  The last 64 will be used by NTPD.
 The 6th field is the number of sample that didn't have valid data ready.
 The 7th field is the number of bad samples.
-The 8th field is the number of times the the mode 1 info was update while nptd was trying to grab a sample.
+The 8th field is the number of times the the mode 1 info was update while NTPD was trying to grab a sample.
 

 
 Here is a sample showing the GPS reception fading out:
@@ -112,6 +130,38 @@ Here is a sample showing the GPS reception fading out:
 54364 85700.160 127.127.28.0  65   0  65   0   0
+

The 'mode' word

+ +

+ Some aspects of the driver behavior can be adjusted by setting bits of + the 'mode' word in the server configuration line:
+   server 127.127.28.x mode Y +

+ + + + + + + + + + + + + + + + + + + + + + +
mode word bits and bit groups
BitDecHexMeaning
011The SHM segment is accessible by the world. (Ignored/rejected for + units 0 and 1!)
1-31--reserved -- do not use
+

Fudge Factors

time1 time @@ -136,9 +186,64 @@ Here is a sample showing the GPS reception fading out:
Not used by this driver.
flag4 0 | 1
If flag4 is set, clockstats records will be written when the driver is polled. -

Additional Information

-

Reference Clock Drivers

+ +

Public vs. Private SHM segments

+ +

The driver attempts to create a shared memory segment with an + identifier depending on the unit number. This identifier (which can be + a numeric value or a string) clearly depends on the method used, which + in turn depends on the host operating system:

+ + + +

There's no support for POSIX shared memory yet.

+ +

NTPD is started as root on most POSIX-like operating systems + and uses the setuid/setgid system API to run under reduced rights once + the initial setup of the process is done. One consequence out of this + is that the allocation of SHM segments must be done early during the + clock setup. The actual polling of the clock is done as the run-time + user; deferring the creation of the SHM segment to this point will + create a SHM segment owned by the runtime-user account. The internal + structure of NTPD does not permit the use of a fudge flag if + this is to be avoided; this is the reason why a mode bit is used for + the configuration of a public segment. +

+ +

When running under Windows, the chosen user account must be able to + create a SHM segment in the global object name space for SHM clocks with + public access. Otherwise the session isolation used by Windows kernels + after WinXP will get into the way if the client program does not run in + the same session. +

+ +

Additional Information

+

Reference Clock Drivers

+
diff --git a/ntpd/refclock_shm.c b/ntpd/refclock_shm.c index 6540e6f37..d528e2bf9 100644 --- a/ntpd/refclock_shm.c +++ b/ntpd/refclock_shm.c @@ -50,6 +50,11 @@ #define NSAMPLES 3 /* stages of median filter */ +/* + * Mode flags + */ +#define MODE_PUBLIC 0x0001 + /* * Function prototypes */ @@ -57,7 +62,6 @@ static int shm_start (int unit, struct peer *peer); static void shm_shutdown (int unit, struct peer *peer); static void shm_poll (int unit, struct peer *peer); static void shm_timer (int unit, struct peer *peer); -static void shm_peek (int unit, struct peer *peer); static void shm_clockstats (int unit, struct peer *peer); static void shm_control (int unit, const struct refclockstat * in_st, struct refclockstat * out_st, struct peer *peer); @@ -100,6 +104,7 @@ struct shmTime { struct shmunit { struct shmTime *shm; /* pointer to shared memory segment */ + int forall; /* access for all UIDs? */ /* debugging/monitoring counters - reset when printed */ int ticks; /* number of attempts to read data*/ @@ -112,76 +117,87 @@ struct shmunit { time_t max_delay; /* age/stale limit */ }; +static struct shmTime* +getShmTime( + int unit, + int/*BOOL*/ forall + ) +{ + struct shmTime *p = NULL; -struct shmTime *getShmTime(int); - -struct shmTime *getShmTime (int unit) { #ifndef SYS_WINNT - int shmid=0; + + int shmid; /* 0x4e545030 is NTP0. * Big units will give non-ascii but that's OK * as long as everybody does it the same way. */ - shmid=shmget (0x4e545030 + unit, sizeof (struct shmTime), - IPC_CREAT | ((unit < 2) ? 0600 : 0666)); + shmid=shmget(0x4e545030 + unit, sizeof (struct shmTime), + IPC_CREAT | (forall ? 0666 : 0600)); if (shmid == -1) { /* error */ msyslog(LOG_ERR, "SHM shmget (unit %d): %m", unit); - return 0; + return NULL; } - else { /* no error */ - struct shmTime *p = (struct shmTime *)shmat (shmid, 0, 0); - if (p == (struct shmTime *)-1) { /* error */ - msyslog(LOG_ERR, "SHM shmat (unit %d): %m", unit); - return 0; - } - return p; + p = (struct shmTime *)shmat (shmid, 0, 0); + if (p == (struct shmTime *)-1) { /* error */ + msyslog(LOG_ERR, "SHM shmat (unit %d): %m", unit); + return NULL; } + return p; + #else - char buf[10]; + + static const char * nspref[2] = { "Local", "Global" }; + char buf[20]; LPSECURITY_ATTRIBUTES psec = 0; HANDLE shmid = 0; SECURITY_DESCRIPTOR sd; SECURITY_ATTRIBUTES sa; + unsigned int numch; - snprintf(buf, sizeof(buf), "NTP%d", unit); - if (unit >= 2) { /* world access */ + numch = snprintf(buf, sizeof(buf), "%s\\NTP%d", + nspref[forall != 0], (unit & 0xFF)); + if (numch >= sizeof(buf)) { + msyslog(LOG_ERR, "SHM name too long (unit %d)", unit); + return NULL; + } + if (forall) { /* world access */ if (!InitializeSecurityDescriptor(&sd, SECURITY_DESCRIPTOR_REVISION)) { msyslog(LOG_ERR,"SHM InitializeSecurityDescriptor (unit %d): %m", unit); - return 0; + return NULL; } - if (!SetSecurityDescriptorDacl(&sd, 1, 0, 0)) { + if (!SetSecurityDescriptorDacl(&sd, TRUE, NULL, FALSE)) { msyslog(LOG_ERR, "SHM SetSecurityDescriptorDacl (unit %d): %m", unit); - return 0; + return NULL; } - sa.nLength=sizeof (SECURITY_ATTRIBUTES); + sa.nLength = sizeof(SECURITY_ATTRIBUTES); sa.lpSecurityDescriptor = &sd; - sa.bInheritHandle = 0; + sa.bInheritHandle = FALSE; psec = &sa; } shmid = CreateFileMapping ((HANDLE)0xffffffff, psec, PAGE_READWRITE, - 0, sizeof (struct shmTime), buf); - if (!shmid) { /*error*/ - char buf[1000]; - + 0, sizeof (struct shmTime), buf); + if (shmid == NULL) { /*error*/ + char buf[1000]; FormatMessage (FORMAT_MESSAGE_FROM_SYSTEM, 0, GetLastError (), 0, buf, sizeof (buf), 0); msyslog(LOG_ERR, "SHM CreateFileMapping (unit %d): %s", unit, buf); - return 0; - } else { - struct shmTime *p = (struct shmTime *) MapViewOfFile (shmid, - FILE_MAP_WRITE, 0, 0, sizeof (struct shmTime)); - if (p == 0) { /*error*/ - char buf[1000]; - - FormatMessage (FORMAT_MESSAGE_FROM_SYSTEM, - 0, GetLastError (), 0, buf, sizeof (buf), 0); - msyslog(LOG_ERR,"SHM MapViewOfFile (unit %d): %s", unit, buf) - return 0; - } - return p; + return NULL; } + p = (struct shmTime *)MapViewOfFile(shmid, FILE_MAP_WRITE, 0, 0, + sizeof (struct shmTime)); + if (p == NULL) { /*error*/ + char buf[1000]; + FormatMessage (FORMAT_MESSAGE_FROM_SYSTEM, + 0, GetLastError (), 0, buf, sizeof (buf), 0); + msyslog(LOG_ERR,"SHM MapViewOfFile (unit %d): %s", unit, buf); + return NULL; + } + #endif + + return p; } /* * shm_start - attach to shared memory @@ -192,18 +208,22 @@ shm_start( struct peer *peer ) { - struct refclockproc *pp; - struct shmunit *up; + struct refclockproc * const pp = peer->procptr; + struct shmunit * const up = emalloc_zero(sizeof(*up)); - pp = peer->procptr; pp->io.clock_recv = noentry; pp->io.srcclock = peer; pp->io.datalen = 0; pp->io.fd = -1; - up = emalloc_zero(sizeof(*up)); + up->forall = (peer->ttl & MODE_PUBLIC) != 0; + if (up->forall && unit < 2) { + msyslog(LOG_WARNING, "SHM: public mode ignored for unit %d", + unit); + up->forall = FALSE; + } - up->shm = getShmTime(unit); + up->shm = getShmTime(unit, up->forall); /* * Initialize miscellaneous peer variables @@ -243,12 +263,12 @@ shm_control( struct peer * peer ) { - struct refclockproc *pp; - struct shmunit *up; - - pp = peer->procptr; - up = pp->unitptr; + struct refclockproc * const pp = peer->procptr; + struct shmunit * const up = pp->unitptr; + UNUSED_ARG(unit); + UNUSED_ARG(in_st); + UNUSED_ARG(out_st); if (NULL == up) return; if (pp->sloppyclockflag & CLK_FLAG1) @@ -269,31 +289,23 @@ shm_shutdown( struct peer *peer ) { - struct refclockproc *pp; - struct shmunit *up; - - pp = peer->procptr; - up = pp->unitptr; + struct refclockproc * const pp = peer->procptr; + struct shmunit * const up = pp->unitptr; + UNUSED_ARG(unit); if (NULL == up) return; #ifndef SYS_WINNT + /* HMS: shmdt() wants char* or const void * */ - (void) shmdt ((char *)up->shm); + (void)shmdt((char *)up->shm); + #else - UnmapViewOfFile (up->shm); -#endif - free(up); -} + UnmapViewOfFile(up->shm); -/* - * shm_timer - called every second - */ -static void -shm_timer(int unit, struct peer *peer) -{ - shm_peek(unit, peer); +#endif + free(up); } @@ -306,13 +318,10 @@ shm_poll( struct peer *peer ) { - struct refclockproc *pp; - struct shmunit *up; + struct refclockproc * const pp = peer->procptr; + struct shmunit * const up = pp->unitptr; int major_error; - pp = peer->procptr; - up = pp->unitptr; - pp->polls++; /* get dominant reason if we have no samples at all */ @@ -345,16 +354,18 @@ shm_poll( } /* - * shm_peek - try to grab a sample + * shm_timer - called onece every second. + * + * This tries to grab a sample from the SHM segment */ static void -shm_peek( +shm_timer( int unit, struct peer *peer ) { - struct refclockproc *pp; - struct shmunit *up; + struct refclockproc * const pp = peer->procptr; + struct shmunit * const up = pp->unitptr; /* access order is important for lock-free SHM access; we ** enforce order by treating the whole structure volatile. @@ -383,19 +394,16 @@ shm_peek( * This is the main routine. It snatches the time from the shm * board and tacks on a local timestamp. */ - pp = peer->procptr; - up = pp->unitptr; up->ticks++; - if (up->shm == 0) { + if ((shm = up->shm) == NULL) { /* try to map again - this may succeed if meanwhile some- body has ipcrm'ed the old (unaccessible) shared mem segment */ - up->shm = getShmTime(unit); - } - shm = up->shm; - if (shm == 0) { - DPRINTF(1, ("%s: no SHM segment\n", - refnumtoa(&peer->srcadr))); - return; + shm = up->shm = getShmTime(unit, up->forall); + if (shm == NULL) { + DPRINTF(1, ("%s: no SHM segment\n", + refnumtoa(&peer->srcadr))); + return; + } } if ( ! shm->valid) { DPRINTF(1, ("%s: SHM not ready\n", @@ -542,28 +550,17 @@ static void shm_clockstats( struct peer *peer ) { - struct refclockproc *pp; - struct shmunit *up; - char logbuf[64]; - unsigned int llen; - - pp = peer->procptr; - up = pp->unitptr; + struct refclockproc * const pp = peer->procptr; + struct shmunit * const up = pp->unitptr; + UNUSED_ARG(unit); if (pp->sloppyclockflag & CLK_FLAG4) { - /* if snprintf() returns a negative values on errors - ** (some older ones do) make sure we are NUL - ** terminated. Using an unsigned result does the trick. - */ - llen = snprintf(logbuf, sizeof(logbuf), - "%3d %3d %3d %3d %3d", - up->ticks, up->good, up->notready, - up->bad, up->clash); - logbuf[min(llen, sizeof(logbuf)-1)] = '\0'; - record_clock_stats(&peer->srcadr, logbuf); + mprintf_clock_stats( + &peer->srcadr, "%3d %3d %3d %3d %3d", + up->ticks, up->good, up->notready, + up->bad, up->clash); } up->ticks = up->good = up->notready = up->bad = up->clash = 0; - } #else