[BUG] fix calls to localtime()

localtime() was called with pointers to tv_sec, which is time_t on
some platforms and long on others. A problem was encountered on
Sparc64 under OpenBSD where tv_sec is long (64 bits) and time_t is
32 bits. Since this architecture is big-endian, it exhibited the
bug because localtime() always worked with the high part of the
value which is always zero. This problem was identified and debugged
by Thierry Fournier.

The correct solution is to pass the date by value and not by pointer,
through an intermediate function. The use of localtime_r() instead of
localtime() also made it possible to get rid of the first call to
localtime() since it does not need to allocate memory anymore.
(cherry picked from commit fe94460d53439b5425c154c657db698c63de3d62)

diff --git a/include/common/standard.h b/include/common/standard.h
index 8efda8a4b46b4194acbc37430c509999d498a548..34d6b4949190a584434457cb7d755e0323cbbec2 100644 (file)
--- a/include/common/standard.h
+++ b/include/common/standard.h
@@ -23,6 +23,7 @@
 #define _COMMON_STANDARD_H
 
 #include <limits.h>
+#include <time.h>
 #include <sys/types.h>
 #include <netinet/in.h>
 #include <common/config.h>
@@ -172,4 +173,14 @@ extern int strl2ic(const char *s, int len);
 extern int strl2irc(const char *s, int len, int *ret);
 extern int strl2llrc(const char *s, int len, long long *ret);
 
+/* This function converts the time_t value <now> into a broken out struct tm
+ * which must be allocated by the caller. It is highly recommended to use this
+ * function intead of localtime() because that one requires a time_t* which
+ * is not always compatible with tv_sec depending on OS/hardware combinations.
+ */
+static inline void get_localtime(const time_t now, struct tm *tm)
+{
+       localtime_r(&now, tm);
+}
+
 #endif /* _COMMON_STANDARD_H */

diff --git a/src/haproxy.c b/src/haproxy.c
index 8cce7ebf71da76cbcad433dafc144dfb8fa7ff73..d211411e1d2b3a9bfaeb080a66c03a398c767824 100644 (file)
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -379,12 +379,7 @@ void init(int argc, char **argv)
        global.rlimit_memmax = HAPROXY_MEMMAX;
 #endif
 
-       /* initialize the libc's localtime structures once for all so that we
-        * won't be missing memory if we want to send alerts under OOM conditions.
-        * Also, the Alert() and Warning() functions need <now> to be initialized.
-        */
        tv_now(&now);
-       localtime((time_t *)&now.tv_sec);
        start_date = now;
 
        init_task();

diff --git a/src/log.c b/src/log.c
index 542dd0dc4c967135e8d79a514dbccc73c10547df..5566d1a449dd36f4612c615419e7e0615eb2293e 100644 (file)
--- a/src/log.c
+++ b/src/log.c
@@ -60,14 +60,14 @@ const char sess_fin_state[8]  = "-RCHDLQT"; /* cliRequest, srvConnect, srvHeader
 void Alert(const char *fmt, ...)
 {
        va_list argp;
-       struct tm *tm;
+       struct tm tm;
 
        if (!(global.mode & MODE_QUIET) || (global.mode & (MODE_VERBOSE | MODE_STARTING))) {
                va_start(argp, fmt);
 
-               tm = localtime((time_t *)&now.tv_sec);
+               get_localtime(now.tv_sec, &tm);
                fprintf(stderr, "[ALERT] %03d/%02d%02d%02d (%d) : ",
-                       tm->tm_yday, tm->tm_hour, tm->tm_min, tm->tm_sec, (int)getpid());
+                       tm.tm_yday, tm.tm_hour, tm.tm_min, tm.tm_sec, (int)getpid());
                vfprintf(stderr, fmt, argp);
                fflush(stderr);
                va_end(argp);
@@ -81,14 +81,14 @@ void Alert(const char *fmt, ...)
 void Warning(const char *fmt, ...)
 {
        va_list argp;
-       struct tm *tm;
+       struct tm tm;
 
        if (!(global.mode & MODE_QUIET) || (global.mode & MODE_VERBOSE)) {
                va_start(argp, fmt);
 
-               tm = localtime((time_t *)&now.tv_sec);
+               get_localtime(now.tv_sec, &tm);
                fprintf(stderr, "[WARNING] %03d/%02d%02d%02d (%d) : ",
-                       tm->tm_yday, tm->tm_hour, tm->tm_min, tm->tm_sec, (int)getpid());
+                       tm.tm_yday, tm.tm_hour, tm.tm_min, tm.tm_sec, (int)getpid());
                vfprintf(stderr, fmt, argp);
                fflush(stderr);
                va_end(argp);
@@ -173,13 +173,15 @@ void send_log(struct proxy *p, int level, const char *message, ...)
 
        if (now.tv_sec != tvsec || dataptr == NULL) {
                /* this string is rebuild only once a second */
-               struct tm *tm = localtime((time_t *)&now.tv_sec);
+               struct tm tm;
+
                tvsec = now.tv_sec;
+               get_localtime(tvsec, &tm);
 
                hdr_len = snprintf(logmsg, sizeof(logmsg),
                                   "<<<<>%s %2d %02d:%02d:%02d %s[%d]: ",
-                                  monthname[tm->tm_mon],
-                                  tm->tm_mday, tm->tm_hour, tm->tm_min, tm->tm_sec,
+                                  monthname[tm.tm_mon],
+                                  tm.tm_mday, tm.tm_hour, tm.tm_min, tm.tm_sec,
                                   progname, pid);
                /* WARNING: depending upon implementations, snprintf may return
                 * either -1 or the number of bytes that would be needed to store
@@ -275,7 +277,7 @@ void tcp_sess_log(struct session *s)
        struct proxy *prx_log;
        int tolog;
        char *svid;
-       struct tm *tm;
+       struct tm tm;
 
        if (s->cli_addr.ss_family == AF_INET)
                inet_ntop(AF_INET,
@@ -286,7 +288,7 @@ void tcp_sess_log(struct session *s)
                          (const void *)&((struct sockaddr_in6 *)(&s->cli_addr))->sin6_addr,
                          pn, sizeof(pn));
 
-       tm = localtime((time_t *)&s->logs.tv_accept.tv_sec);
+       get_localtime(s->logs.tv_accept.tv_sec, &tm);
 
        if (fe->logfac1 >= 0)
                prx_log = fe;
@@ -312,8 +314,8 @@ void tcp_sess_log(struct session *s)
                 (s->cli_addr.ss_family == AF_INET) ?
                 ntohs(((struct sockaddr_in *)&s->cli_addr)->sin_port) :
                 ntohs(((struct sockaddr_in6 *)&s->cli_addr)->sin6_port),
-                tm->tm_mday, monthname[tm->tm_mon], tm->tm_year+1900,
-                tm->tm_hour, tm->tm_min, tm->tm_sec, s->logs.tv_accept.tv_usec/1000,
+                tm.tm_mday, monthname[tm.tm_mon], tm.tm_year+1900,
+                tm.tm_hour, tm.tm_min, tm.tm_sec, s->logs.tv_accept.tv_usec/1000,
                 fe->id, be->id, svid,
                 (s->logs.t_queue >= 0) ? s->logs.t_queue : -1,
                 (s->logs.t_connect >= 0) ? s->logs.t_connect - s->logs.t_queue : -1,

diff --git a/src/proto_http.c b/src/proto_http.c
index f94b01c16e4b19c2ee7abb1e61aa851d171785e6..50520eff273ba65f7862a74a8fb9c2f0de7093cf 100644 (file)
--- a/src/proto_http.c
+++ b/src/proto_http.c
@@ -745,7 +745,7 @@ static void http_sess_log(struct session *s)
        int tolog;
        char *uri, *h;
        char *svid;
-       struct tm *tm;
+       struct tm tm;
        static char tmpline[MAX_SYSLOG_LEN];
        int hdr;
 
@@ -762,8 +762,7 @@ static void http_sess_log(struct session *s)
                          (const void *)&((struct sockaddr_in6 *)(&s->cli_addr))->sin6_addr,
                          pn, sizeof(pn));
 
-       tm = localtime((time_t *)&s->logs.tv_accept.tv_sec);
-
+       get_localtime(s->logs.tv_accept.tv_sec, &tm);
 
        /* FIXME: let's limit ourselves to frontend logging for now. */
        tolog = fe->to_log;
@@ -821,8 +820,8 @@ static void http_sess_log(struct session *s)
                 (s->cli_addr.ss_family == AF_INET) ?
                 ntohs(((struct sockaddr_in *)&s->cli_addr)->sin_port) :
                 ntohs(((struct sockaddr_in6 *)&s->cli_addr)->sin6_port),
-                tm->tm_mday, monthname[tm->tm_mon], tm->tm_year+1900,
-                tm->tm_hour, tm->tm_min, tm->tm_sec, s->logs.tv_accept.tv_usec/1000,
+                tm.tm_mday, monthname[tm.tm_mon], tm.tm_year+1900,
+                tm.tm_hour, tm.tm_min, tm.tm_sec, s->logs.tv_accept.tv_usec/1000,
                 fe->id, be->id, svid,
                 s->logs.t_request,
                 (s->logs.t_queue >= 0) ? s->logs.t_queue - s->logs.t_request : -1,

diff --git a/tests/test-log.cfg b/tests/test-log.cfg
new file mode 100644 (file)
index 0000000..a8d971c
--- /dev/null
+++ b/tests/test-log.cfg
@@ -0,0 +1,24 @@
+# This is a test configuration.
+# Its purpose is simply to emit logs on the loopback in order to verify
+# that the time is correct. To be used with tcpdump on lo, or with
+# "strace -s100 -esendto".
+
+global
+       log 127.0.0.1:514 local0
+
+defaults
+       balance uri
+       contimeout 5000
+       clitimeout 5000
+       srvtimeout 5000
+
+listen  http
+       log global
+       log 127.0.0.1:514 local0
+       option httplog
+        mode            http
+        bind           :8000
+        option          dontlognull
+       balance roundrobin
+       server  tux 127.0.0.1:65535 check inter 1000
+