We deviate from RFC3164 which states timestamp should be in localtime
because although our __localtime_r does not set tzname, the relay still
might still use localtime (which does) and if the server timezone changes
it might result in wrong timestamp from client. It still does not help
if a process switches its TZ setting from a timezone that has leap
seconds, to one that doesn't (or vice versa), but this would incur in
other problems.
It also handles the highly unlikely case where gmtime might return NULL,
in this case only the PRI is set to hopefully instruct the relay to
get eh TIMESTAMP (as defined by the RFC).
Finally it also uses internally the 64 bit time_t interfaces (to avoid
y2038 issues on 32 bit legacy architectures).
Checked on x86_64-linux-gnu and i686-linux-gnu.
---
misc/syslog.c | 39 ++++++++++++++++++++++++++++++++-------
1 file changed, 32 insertions(+), 7 deletions(-)
On Mär 18 2022, Adhemerval Zanella via Libc-alpha wrote:
> diff --git a/misc/syslog.c b/misc/syslog.c
> index 7852441615..997e423228 100644
> --- a/misc/syslog.c
> +++ b/misc/syslog.c
> @@ -161,11 +161,25 @@ __vsyslog_internal(int pri, const char *fmt, va_list ap,
>
> /* "%h %e %H:%M:%S " */
> char timestamp[timestamp_size];
> - time_t now = time_now ();
> + __time64_t now = time64_now ();
> +
> + /* We deviate from RFC3164 which states timestamp should be in localtime
> + because although our __localtime_r does not set tzname, the relay still
> + might still use localtime (which does) and if the server timezone changes
s/still...still/still/
@@ -161,11 +161,25 @@ __vsyslog_internal(int pri, const char *fmt, va_list ap,
/* "%h %e %H:%M:%S " */
char timestamp[timestamp_size];
- time_t now = time_now ();
+ __time64_t now = time64_now ();
+
+ /* We deviate from RFC3164 which states timestamp should be in localtime
+ because although our __localtime_r does not set tzname, the relay still
+ might still use localtime (which does) and if the server timezone changes
+ it might result in wrong timestamp from client. It still does not help
+ if a process switches its TZ setting from a timezone that has leap
+ seconds, to one that doesn't (or vice versa), but this would incur in
+ other problems. */
struct tm now_tm;
- __localtime_r (&now, &now_tm);
- __strftime_l (timestamp, sizeof timestamp, "%h %e %T ", &now_tm,
- _nl_C_locobj_ptr);
+ bool has_ts = __gmtime64_r (&now, &now_tm) != NULL;
+
+ /* In the highly unlike case of gmtime_r failure (the clock being
+ INT_MIN + 1900 or follow INT_MAX + 1900) we skip the hostname so the
+ message is handl as valid PRI but without TIMESTAMP or invalid TIMESTAMP
+ (which should force the relay to add the timestamp itself). */
+ if (has_ts)
+ __strftime_l (timestamp, sizeof timestamp, "%h %e %T ", &now_tm,
+ _nl_C_locobj_ptr);
#define SYSLOG_HEADER(__pri, __timestamp, __msgoff, pid) \
"<%d>%s %n%s%s%.0d%s: ", \
@@ -173,10 +187,18 @@ __vsyslog_internal(int pri, const char *fmt, va_list ap,
LogTag == NULL ? __progname : LogTag, \
pid != 0 ? "[" : "", pid, pid != 0 ? "]" : ""
+#define SYSLOG_HEADER_WITHOUT_TS(__pri, __msgoff) \
+ "<%d>: %n", __pri, __msgoff
+
/* Try to use a static buffer as an optimization. */
char bufs[bufs_size];
- int l = __snprintf (bufs, sizeof bufs,
- SYSLOG_HEADER (pri, timestamp, &msgoff, pid));
+ int l;
+ if (has_ts)
+ l = __snprintf (bufs, sizeof bufs,
+ SYSLOG_HEADER (pri, timestamp, &msgoff, pid));
+ else
+ l = __snprintf (bufs, sizeof bufs,
+ SYSLOG_HEADER_WITHOUT_TS (pri, &msgoff));
if (l < sizeof (bufs))
{
va_list apc;
@@ -204,7 +226,10 @@ __vsyslog_internal(int pri, const char *fmt, va_list ap,
if (f != NULL)
{
__fsetlocking (f, FSETLOCKING_BYCALLER);
- fprintf (f, SYSLOG_HEADER (pri, timestamp, &msgoff, pid));
+ if (has_ts)
+ fprintf (f, SYSLOG_HEADER (pri, timestamp, &msgoff, pid));
+ else
+ fprintf (f, SYSLOG_HEADER_WITHOUT_TS (pri, &msgoff));
/* Restore errno for %m format. */
__set_errno (saved_errno);
__vfprintf_internal (f, fmt, ap, mode_flags);