Looking at the code, the vast majority of cases there is indeed a call that looks like this:

msyslog(LOG_ERR...

However, there are some that look like this:

fprintf(stderr...

One that looks like this:

mfprintf(stderr...

One that just calls fputs. And one case that doesn't print anything.

I think that most of the fprintf calls are when the simulator exits, which might be appropriate. Any thoughts on the others?

Created attachment 1014 [details]
Fix exit from panicgate threshold without message

This fixes exit without message from the panicgate threshold being exceeded.

lib/isc/unix/app.c
lib/isc/win32/app.c
lib/isc/win32/socket.c - InitSockets()
libntp/a_md5encrypt.c: logs, but no: - EXITING
libntp/emalloc.c: logs with 'fatal ...' instead of '- EXITING'
libntp/iosignal.c: logs, no EXITING; several places
libntp/netof.c: logs, no EXITING
libntp/ntp_intres.c: logs, no EXITING
libntp/ntp_worker.c - check it out.
libntp/socket.c: logs, no EXITING; several places
libntp/syssignal.c: calls perror()
libntp/systime.c: logs, no EXITING
libntp/work_fork.c: logs, no EXITING
libntp/work_thread.c: logs, no EXITING
ntpd/cmd_args.c: logs with fatal, not EXITING
ntpd/ntp_config.c: logs with other messages, not EXITING (several places)
ntpd/ntp_crypto.c: nonstandard messages
ntpd/ntp_io.c: nonstandard messages
ntpd/ntp_proto.c: nonstandard messages
ntpd/ntp_scanner.c: nonstandard messages
ntpd/ntp_timer.c: nonstandard messages
ntpd/ntpd.c: nonstandard messages
ntpd/ntpd-opts.c: (I think) nonstandard messages.  .def file?
ntpd/ntpdsim-opts.c: nonstandard messages.  .def file?
ntpd/refclock_chu.c: nonstandard messages
ntpd/refclock_mx4200.c: nonstandard messages
ntpdc/ntpdc-opts.c: nonstandard messages.  .def file?
ntpdc/ntpdc.c: nonstandard messages
ntpq/ntpq-opts.c: nonstandard messages.  .def file?
ntpq/ntpq-subs.c: nonstandard messages
ntpq/ntpq.c: nonstandard messages
ntpsnmpd/ntpsnmpd-opts.c: nonstandard messages.  .def file?
ntpsnmpd/ntpsnmpd.c: nonstandard messages.
ports/winnt/libntp/wun32_io.c: nonstandard messages
ports/winnt/ntpd/nt_clockstuff.c: nonstandard messages
ports/winnt/ntpd/ntp_iocompletionport.c: nonstandard messages
ports/winnt/ntpd/ntservice.c: nonstandard messages
sntp/main.c: nonstandard messages
sntp/sntp-opts.c: nonstandard message.  .def file?

We might want: "function() - EXITING: ..." as the common format for these.

Look at:
 sntp/libevent/log.c
 sntp/libopts/alias.c
 sntp/libopts/autoopts.c
 sntp/libopts/genshell.c (do we care about this one?)
 sntp/libopts/makeshell.c (do we care about this one?)
 sntp/libopts/restore.c
do we want to think about a hook?

Also see bug 1943 and bug 2049.

I remember a bug about making sure we use consistent and correct exit status values, but I'm not seeing that now.

On a related note that should be dealt with at the same time, NTP should log ERROR level messages at least to the syslog if not all the logging destinations. I just realized that if you have logging to a file on then you will not see error in the syslog. Since logging allows you to specify exactly what you want to log, it is very possible that error conditions will never be seen by anyone.

I'm game to have this issue block 4.2.8.

Thoughts, anybody?

In the source, the panic_stop message is treated much differently from most ntpd messages associated with a fatal condition, in two ways.

(1) Messages associated with most fatal conditions are logged, without filtering, before the exit() call. But panic_stop is logged only when the syslog filter has "+sysevents". And that's not the out-of-box filter setting, so it's filtered out. The out-of-box filter setting is "logconfig =syncall".

(2) Messages associated with most fatal conditions have a syslog severity of 3 "error". But panic_stop messages have a much lower syslog severity of 6 "info". That is lower even than "notice" which means "normal but significant condition". An exit() call does not in any useful way leave the ntpd daemon in a "normal but significant condition".

POSSIBLE CHANGES

1. Given the importance of catching fatal conditions, "logconfig =allall" could be made the out-of-box default, not "logconfig =syncall". This wouldn't be foolproof, though. Distribution packages generally preinstall a basic ntp.conf, and this might well include a well-intentioned "logconfig" command.

2. The report_event() routine used to log the panic_stop condition does not take a severity as an argument. Instead it logs all sysevents at severity "info". Adding a severity argument to report_event() would permit raising the severity to "error" on the panic_stop condition. But implementation would be kind of a big deal because report_event() is called in so many places.

3. Another solution would be to use msyslog() instead of report_event() to log a panic_stop. This would make it trivial to log the panic_stop at severity "error". Plus, this makes it unnecessary to change the default "logconfig" mask. The drawback is that the panic_stop condition would no longer be recorded as a statistics record. That creates a compatibility issue with any existing scripts that look for it, either in protostats or in syslog records.

The safest option -- the option I actually propose -- is to add a call to msyslog() after the report_event(), so the sequence of events in ntp_loopfilter.c would be:

/* log clock statistics and optionally copy syslog */
report_event(EVNT_SYSFAULT, NULL, tbuf);

/* always log fatal condition to syslog */
msyslog(LOG_ERR, "Fatal clock panic condition");

/* pass back the fatal return code */
return (-1);

This solution might log two events to syslog, and that's not actually a bad thing.

Brian Utterback's patch attached to this bug actually looks perfect.

I'm applying Brian's patch for 4.2.8p16

Harlan, this is ready (along with a number of other fixes) in

bk://stenn.ntp.org/../hart/ntp-stable-3784

STAGED for ntp-4.2.8p16.

Thanks, folks!

The attached patch is not quite right.  Brian provided an updated patch after I noticed a missing %s arg.

Harlan, please pull:

~hart/ntp-stable-2410

Done, thanks Dave!

STAGED and released in ntp-4.2.8p16.

Thanks, folks!

Brian, please mark this bug as VERIFIED or REOPENED, as appropriate.