Fix putenv race conditions with undefined behavior.

Do all putenv calls before Emacs creates any threads.
Use a safer way to modify the TZ environment variable in the
presence of multiple threads.  For further thread-safety,
prefer localtime_r and gmtime_r to localtime and gmtime,
and prefer struct tm's tm_gmtoff (if available) to calling
both localtime_r and gmtime_r.
* configure.ac (LOCALTIME_CACHE): Remove.
We needn't worry about SunOS 4 any more; Sun dropped support in 2003.
All uses of LOCALTIME_CACHE removed.  This simplifies the fix.
(tzalloc): Add check for this function.
* admin/merge-gnulib (GNULIB_MODULES): Add time_r, since Emacs now
calls localtime_r and gmtime_r directly.
* src/dbusbind.c (Fdbus__init_bus): Move xputenv call from here ...
(init_dbusbind): ... to this new function.
* src/emacs.c (main) [HAVE_DBUS]: Call it before creating threads.
* src/xterm.c (x_term_init): Move xputenv call from here ...
(init_xterm): ... to this new function.
* src/emacs.c (main) [USE_GTK]: Call it before creating threads.
* src/editfns.c (HAVE_TM_GMTOFF): Default to false.
(dump_tz_string): New constant.
(init_editfns): Use it.  This centralizes the dump_tz stuff.
Call set_time_zone_rule here, so that its xputenv is done
before Emacs goes multithreaded.
(mktime_z) [!HAVE_TZALLOC]: New function, which is typically
thread-safe enough for Emacs.
(format_time_string, Fdecode_time, Fcurrent_time_string)
(Fcurrent_time_zone):
Prefer localtime_r and gmtime_r, which are more thread-safe, to
localtime and gmtime.  Remove now-unnecessary calls to block_input.
(tm_gmtoff): New static function.
(Fdecode_time, Fcurrent_time_zone): Use it.
(Fencode_time): Use mktime_z, for better thread-safety.
(set_time_zone_rule): Now static.  Rewrite to be mostly thread-safe,
i.e., not quite thread-safe but good enough for Emacs typical usage.
Do not reclaim storage that is in the environment; let it leak.
Always call tzset, since localtime_r does not.
* src/emacs.c (dump_tz, Fdump_emacs) [HAVE_TZSET]: Remove dump_tz stuff.
This is now done in init_editfns.
* src/systime.h (mktime_z, timezone_t, tzalloc, tzfree) [!HAVE_TZALLOC]:
New macros and declarations, for platforms lacking tzalloc & friends.

Fixes: debbugs:8705

1 files changed, 172 insertions, 187 deletions

diff --git a/src/editfns.c b/src/editfns.c
index e7c960dfffe..d17d809c9ec 100644
--- a/src/editfns.c
+++ b/src/editfns.c
@@ -64,11 +64,17 @@ along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.  */
 extern Lisp_Object w32_get_internal_run_time (void);
 #endif
 
+static void set_time_zone_rule (char const *);
 static Lisp_Object format_time_string (char const *, ptrdiff_t, struct timespec,
 				       bool, struct tm *);
+static long int tm_gmtoff (struct tm *);
 static int tm_diff (struct tm *, struct tm *);
 static void update_buffer_properties (ptrdiff_t, ptrdiff_t);
 
+#ifndef HAVE_TM_GMTOFF
+# define HAVE_TM_GMTOFF false
+#endif
+
 static Lisp_Object Qbuffer_access_fontify_functions;
 
 /* Symbol for the text property used to mark fields.  */
@@ -79,15 +85,12 @@ Lisp_Object Qfield;
 
 static Lisp_Object Qboundary;
 
-/* The startup value of the TZ environment variable so it can be
-   restored if the user calls set-time-zone-rule with a nil
-   argument.  If null, the TZ environment variable was unset.  */
+/* The startup value of the TZ environment variable; null if unset.  */
 static char const *initial_tz;
 
-/* True if the static variable tzvalbuf (defined in
-   set_time_zone_rule) is part of 'environ'.  */
-static bool tzvalbuf_in_environ;
-
+/* A valid but unlikely setting for the TZ environment variable.
+   It is OK (though a bit slower) if the user chooses this value.  */
+static char const dump_tz_string[] = "TZ=UtC0";
 
 void
 init_editfns (void)
@@ -101,13 +104,38 @@ init_editfns (void)
   init_system_name ();
 
 #ifndef CANNOT_DUMP
-  /* Don't bother with this on initial start when just dumping out */
+  /* When just dumping out, set the time zone to a known unlikely value
+     and skip the rest of this function.  */
   if (!initialized)
-    return;
-#endif /* not CANNOT_DUMP */
+    {
+# ifdef HAVE_TZSET
+      xputenv ((char *) dump_tz_string);
+      tzset ();
+# endif
+      return;
+    }
+#endif
+
+  char *tz = getenv ("TZ");
+  initial_tz = tz;
 
-  initial_tz = getenv ("TZ");
-  tzvalbuf_in_environ = 0;
+#if !defined CANNOT_DUMP && defined HAVE_TZSET
+  /* If the execution TZ happens to be the same as the dump TZ,
+     change it to some other value and then change it back,
+     to force the underlying implementation to reload the TZ info.
+     This is needed on implementations that load TZ info from files,
+     since the TZ file contents may differ between dump and execution.  */
+  if (tz && strcmp (tz, &dump_tz_string[sizeof "TZ=" - 1]) == 0)
+    {
+      ++*tz;
+      tzset ();
+      --*tz;
+    }
+#endif
+
+  /* Call set_time_zone_rule now, so that its call to putenv is done
+     before multiple threads are active.  */
+  set_time_zone_rule (tz);
 
   pw = getpwuid (getuid ());
 #ifdef MSDOS
@@ -1373,6 +1401,30 @@ time_overflow (void)
   error ("Specified time is not representable");
 }
 
+/* A substitute for mktime_z on platforms that lack it.  It's not
+   thread-safe, but should be good enough for Emacs in typical use.  */
+#ifndef HAVE_TZALLOC
+time_t
+mktime_z (timezone_t tz, struct tm *tm)
+{
+  char *oldtz = getenv ("TZ");
+  USE_SAFE_ALLOCA;
+  if (oldtz)
+    {
+      size_t oldtzsize = strlen (oldtz) + 1;
+      char *oldtzcopy = SAFE_ALLOCA (oldtzsize);
+      oldtz = strcpy (oldtzcopy, oldtz);
+    }
+  block_input ();
+  set_time_zone_rule (tz);
+  time_t t = mktime (tm);
+  set_time_zone_rule (oldtz);
+  unblock_input ();
+  SAFE_FREE ();
+  return t;
+}
+#endif
+
 /* Return the upper part of the time T (everything but the bottom 16 bits).  */
 static EMACS_INT
 hi_time (time_t t)
@@ -1768,39 +1820,28 @@ format_time_string (char const *format, ptrdiff_t formatlen,
   size_t len;
   Lisp_Object bufstring;
   int ns = t.tv_nsec;
-  struct tm *tm;
   USE_SAFE_ALLOCA;
 
-  while (1)
-    {
-      time_t *taddr = &t.tv_sec;
-      block_input ();
-
-      synchronize_system_time_locale ();
-
-      tm = ut ? gmtime (taddr) : localtime (taddr);
-      if (! tm)
-	{
-	  unblock_input ();
-	  time_overflow ();
-	}
-      *tmp = *tm;
+  tmp = ut ? gmtime_r (&t.tv_sec, tmp) : localtime_r (&t.tv_sec, tmp);
+  if (! tmp)
+    time_overflow ();
+  synchronize_system_time_locale ();
 
+  while (true)
+    {
       buf[0] = '\1';
-      len = emacs_nmemftime (buf, size, format, formatlen, tm, ut, ns);
+      len = emacs_nmemftime (buf, size, format, formatlen, tmp, ut, ns);
       if ((0 < len && len < size) || (len == 0 && buf[0] == '\0'))
 	break;
 
       /* Buffer was too small, so make it bigger and try again.  */
-      len = emacs_nmemftime (NULL, SIZE_MAX, format, formatlen, tm, ut, ns);
-      unblock_input ();
+      len = emacs_nmemftime (NULL, SIZE_MAX, format, formatlen, tmp, ut, ns);
       if (STRING_BYTES_BOUND <= len)
 	string_overflow ();
       size = len + 1;
       buf = SAFE_ALLOCA (size);
     }
 
-  unblock_input ();
   bufstring = make_unibyte_string (buf, len);
   SAFE_FREE ();
   return code_convert_string_norecord (bufstring, Vlocale_coding_system, 0);
@@ -1824,38 +1865,30 @@ DOW and ZONE.)  */)
   (Lisp_Object specified_time)
 {
   time_t time_spec = lisp_seconds_argument (specified_time);
-  struct tm save_tm;
-  struct tm *decoded_time;
-  Lisp_Object list_args[9];
+  struct tm local_tm, gmt_tm;
 
-  block_input ();
-  decoded_time = localtime (&time_spec);
-  if (decoded_time)
-    save_tm = *decoded_time;
-  unblock_input ();
-  if (! (decoded_time
-	 && MOST_NEGATIVE_FIXNUM - TM_YEAR_BASE <= save_tm.tm_year
-	 && save_tm.tm_year <= MOST_POSITIVE_FIXNUM - TM_YEAR_BASE))
+  if (! (localtime_r (&time_spec, &local_tm)
+	 && MOST_NEGATIVE_FIXNUM - TM_YEAR_BASE <= local_tm.tm_year
+	 && local_tm.tm_year <= MOST_POSITIVE_FIXNUM - TM_YEAR_BASE))
     time_overflow ();
-  XSETFASTINT (list_args[0], save_tm.tm_sec);
-  XSETFASTINT (list_args[1], save_tm.tm_min);
-  XSETFASTINT (list_args[2], save_tm.tm_hour);
-  XSETFASTINT (list_args[3], save_tm.tm_mday);
-  XSETFASTINT (list_args[4], save_tm.tm_mon + 1);
-  /* On 64-bit machines an int is narrower than EMACS_INT, thus the
-     cast below avoids overflow in int arithmetics.  */
-  XSETINT (list_args[5], TM_YEAR_BASE + (EMACS_INT) save_tm.tm_year);
-  XSETFASTINT (list_args[6], save_tm.tm_wday);
-  list_args[7] = save_tm.tm_isdst ? Qt : Qnil;
 
-  block_input ();
-  decoded_time = gmtime (&time_spec);
-  if (decoded_time == 0)
-    list_args[8] = Qnil;
-  else
-    XSETINT (list_args[8], tm_diff (&save_tm, decoded_time));
-  unblock_input ();
-  return Flist (9, list_args);
+  /* Avoid overflow when INT_MAX < EMACS_INT_MAX.  */
+  EMACS_INT tm_year_base = TM_YEAR_BASE;
+
+  return Flist (9, ((Lisp_Object [])
+		    {make_number (local_tm.tm_sec),
+		     make_number (local_tm.tm_min),
+		     make_number (local_tm.tm_hour),
+		     make_number (local_tm.tm_mday),
+		     make_number (local_tm.tm_mon + 1),
+		     make_number (local_tm.tm_year + tm_year_base),
+		     make_number (local_tm.tm_wday),
+		     local_tm.tm_isdst ? Qt : Qnil,
+		     (HAVE_TM_GMTOFF
+		      ? make_number (tm_gmtoff (&local_tm))
+		      : gmtime_r (&time_spec, &gmt_tm)
+		      ? make_number (tm_diff (&local_tm, &gmt_tm))
+		      : Qnil)}));
 }
 
 /* Return OBJ - OFFSET, checking that OBJ is a valid fixnum and that
@@ -1911,18 +1944,12 @@ usage: (encode-time SECOND MINUTE HOUR DAY MONTH YEAR &optional ZONE)  */)
   if (CONSP (zone))
     zone = XCAR (zone);
   if (NILP (zone))
-    {
-      block_input ();
-      value = mktime (&tm);
-      unblock_input ();
-    }
+    value = mktime (&tm);
   else
     {
       static char const tzbuf_format[] = "XXX%s%"pI"d:%02d:%02d";
       char tzbuf[sizeof tzbuf_format + INT_STRLEN_BOUND (EMACS_INT)];
-      char *old_tzstring;
       const char *tzstring;
-      USE_SAFE_ALLOCA;
 
       if (EQ (zone, Qt))
 	tzstring = "UTC0";
@@ -1939,29 +1966,13 @@ usage: (encode-time SECOND MINUTE HOUR DAY MONTH YEAR &optional ZONE)  */)
 	  tzstring = tzbuf;
 	}
       else
-	error ("Invalid time zone specification");
+	tzstring = 0;
 
-      old_tzstring = getenv ("TZ");
-      if (old_tzstring)
-	{
-	  char *buf = SAFE_ALLOCA (strlen (old_tzstring) + 1);
-	  old_tzstring = strcpy (buf, old_tzstring);
-	}
-
-      block_input ();
-
-      /* Set TZ before calling mktime; merely adjusting mktime's returned
-	 value doesn't suffice, since that would mishandle leap seconds.  */
-      set_time_zone_rule (tzstring);
-
-      value = mktime (&tm);
-
-      set_time_zone_rule (old_tzstring);
-#ifdef LOCALTIME_CACHE
-      tzset ();
-#endif
-      unblock_input ();
-      SAFE_FREE ();
+      timezone_t tz = tzstring ? tzalloc (tzstring) : 0;
+      if (! tz)
+	error ("Invalid time zone specification");
+      value = mktime_z (tz, &tm);
+      tzfree (tz);
     }
 
   if (value == (time_t) -1)
@@ -1987,34 +1998,27 @@ but this is considered obsolete.  */)
   (Lisp_Object specified_time)
 {
   time_t value = lisp_seconds_argument (specified_time);
-  struct tm *tm;
-  char buf[sizeof "Mon Apr 30 12:49:17 " + INT_STRLEN_BOUND (int) + 1];
-  int len IF_LINT (= 0);
 
   /* Convert to a string in ctime format, except without the trailing
      newline, and without the 4-digit year limit.  Don't use asctime
      or ctime, as they might dump core if the year is outside the
      range -999 .. 9999.  */
-  block_input ();
-  tm = localtime (&value);
-  if (tm)
-    {
-      static char const wday_name[][4] =
-	{ "Sun", "Mon", "Tue", "Wed", "Thu", "Fri", "Sat" };
-      static char const mon_name[][4] =
-	{ "Jan", "Feb", "Mar", "Apr", "May", "Jun",
-	  "Jul", "Aug", "Sep", "Oct", "Nov", "Dec" };
-      printmax_t year_base = TM_YEAR_BASE;
-
-      len = sprintf (buf, "%s %s%3d %02d:%02d:%02d %"pMd,
-		     wday_name[tm->tm_wday], mon_name[tm->tm_mon], tm->tm_mday,
-		     tm->tm_hour, tm->tm_min, tm->tm_sec,
-		     tm->tm_year + year_base);
-    }
-  unblock_input ();
-  if (! tm)
+  struct tm tm;
+  if (! localtime_r (&value, &tm))
     time_overflow ();
 
+  static char const wday_name[][4] =
+    { "Sun", "Mon", "Tue", "Wed", "Thu", "Fri", "Sat" };
+  static char const mon_name[][4] =
+    { "Jan", "Feb", "Mar", "Apr", "May", "Jun",
+      "Jul", "Aug", "Sep", "Oct", "Nov", "Dec" };
+  printmax_t year_base = TM_YEAR_BASE;
+  char buf[sizeof "Mon Apr 30 12:49:17 " + INT_STRLEN_BOUND (int) + 1];
+  int len = sprintf (buf, "%s %s%3d %02d:%02d:%02d %"pMd,
+		     wday_name[tm.tm_wday], mon_name[tm.tm_mon], tm.tm_mday,
+		     tm.tm_hour, tm.tm_min, tm.tm_sec,
+		     tm.tm_year + year_base);
+
   return make_unibyte_string (buf, len);
 }
 
@@ -2041,6 +2045,17 @@ tm_diff (struct tm *a, struct tm *b)
 	  + (a->tm_sec - b->tm_sec));
 }
 
+/* Yield A's UTC offset, or an unspecified value if unknown.  */
+static long int
+tm_gmtoff (struct tm *a)
+{
+#if HAVE_TM_GMTOFF
+  return a->tm_gmtoff;
+#else
+  return 0;
+#endif
+}
+
 DEFUN ("current-time-zone", Fcurrent_time_zone, Scurrent_time_zone, 0, 1, 0,
        doc: /* Return the offset and name for the local time zone.
 This returns a list of the form (OFFSET NAME).
@@ -2059,32 +2074,30 @@ the data it can't find.  */)
   (Lisp_Object specified_time)
 {
   struct timespec value;
-  int offset;
-  struct tm *t;
-  struct tm localtm;
+  struct tm local_tm, gmt_tm;
   Lisp_Object zone_offset, zone_name;
 
   zone_offset = Qnil;
   value = make_timespec (lisp_seconds_argument (specified_time), 0);
-  zone_name = format_time_string ("%Z", sizeof "%Z" - 1, value, 0, &localtm);
-  block_input ();
-  t = gmtime (&value.tv_sec);
-  if (t)
-    offset = tm_diff (&localtm, t);
-  unblock_input ();
+  zone_name = format_time_string ("%Z", sizeof "%Z" - 1, value, 0, &local_tm);
 
-  if (t)
+  if (HAVE_TM_GMTOFF || gmtime_r (&value.tv_sec, &gmt_tm))
     {
+      long int offset = (HAVE_TM_GMTOFF
+			 ? tm_gmtoff (&local_tm)
+			 : tm_diff (&local_tm, &gmt_tm));
       zone_offset = make_number (offset);
       if (SCHARS (zone_name) == 0)
 	{
 	  /* No local time zone name is available; use "+-NNNN" instead.  */
-	  int m = offset / 60;
-	  int am = offset < 0 ? - m : m;
-	  char buf[sizeof "+00" + INT_STRLEN_BOUND (int)];
-	  zone_name = make_formatted_string (buf, "%c%02d%02d",
+	  long int m = offset / 60;
+	  long int am = offset < 0 ? - m : m;
+	  long int hour = am / 60;
+	  int min = am % 60;
+	  char buf[sizeof "+00" + INT_STRLEN_BOUND (long int)];
+	  zone_name = make_formatted_string (buf, "%c%02ld%02d",
 					     (offset < 0 ? '-' : '+'),
-					     am / 60, am % 60);
+					     hour, min);
 	}
     }
 
@@ -2123,12 +2136,12 @@ only the former.  */)
 
 /* Set the local time zone rule to TZSTRING.
 
-   This function is not thread-safe, partly because putenv, unsetenv
-   and tzset are not, and partly because of the static storage it
-   updates.  Other threads that invoke localtime etc. may be adversely
-   affected while this function is executing.  */
+   This function is not thread-safe, in theory because putenv is not,
+   but mostly because of the static storage it updates.  Other threads
+   that invoke localtime etc. may be adversely affected while this
+   function is executing.  */
 
-void
+static void
 set_time_zone_rule (const char *tzstring)
 {
   /* A buffer holding a string of the form "TZ=value", intended
@@ -2137,75 +2150,47 @@ set_time_zone_rule (const char *tzstring)
   static ptrdiff_t tzvalbufsize;
 
   int tzeqlen = sizeof "TZ=" - 1;
+  ptrdiff_t tzstringlen = tzstring ? strlen (tzstring) : 0;
+  char *tzval = tzvalbuf;
+  bool new_tzvalbuf = tzvalbufsize <= tzeqlen + tzstringlen;
 
-#ifdef LOCALTIME_CACHE
-  /* These two values are known to load tz files in buggy implementations,
-     i.e., Solaris 1 executables running under either Solaris 1 or Solaris 2.
-     Their values shouldn't matter in non-buggy implementations.
-     We don't use string literals for these strings,
-     since if a string in the environment is in readonly
-     storage, it runs afoul of bugs in SVR4 and Solaris 2.3.
-     See Sun bugs 1113095 and 1114114, ``Timezone routines
-     improperly modify environment''.  */
-
-  static char set_time_zone_rule_tz[][sizeof "TZ=GMT+0"]
-    = { "TZ=GMT+0", "TZ=GMT+1" };
-
-  /* In SunOS 4.1.3_U1 and 4.1.4, if TZ has a value like
-     "US/Pacific" that loads a tz file, then changes to a value like
-     "XXX0" that does not load a tz file, and then changes back to
-     its original value, the last change is (incorrectly) ignored.
-     Also, if TZ changes twice in succession to values that do
-     not load a tz file, tzset can dump core (see Sun bug#1225179).
-     The following code works around these bugs.  */
+  if (new_tzvalbuf)
+    {
+      /* Do not attempt to free the old tzvalbuf, since another thread
+	 may be using it.  In practice, the first allocation is large
+	 enough and memory does not leak.  */
+      tzval = xpalloc (NULL, &tzvalbufsize,
+		       tzeqlen + tzstringlen - tzvalbufsize + 1, -1, 1);
+      tzvalbuf = tzval;
+      tzval[1] = 'Z';
+      tzval[2] = '=';
+    }
 
   if (tzstring)
     {
-      /* Temporarily set TZ to a value that loads a tz file
-	 and that differs from tzstring.  */
-      bool eq0 = strcmp (tzstring, set_time_zone_rule_tz[0] + tzeqlen) == 0;
-      xputenv (set_time_zone_rule_tz[eq0]);
+      /* Modify TZVAL in place.  Although this is dicey in a
+	 multithreaded environment, we know of no portable alternative.
+	 Calling putenv or setenv could crash some other thread.  */
+      tzval[0] = 'T';
+      strcpy (tzval + tzeqlen, tzstring);
     }
   else
     {
-      /* The implied tzstring is unknown, so temporarily set TZ to
-	 two different values that each load a tz file.  */
-      xputenv (set_time_zone_rule_tz[0]);
-      tzset ();
-      xputenv (set_time_zone_rule_tz[1]);
+      /* Turn 'TZ=whatever' into an empty environment variable 'tZ='.
+	 Although this is also dicey, calling unsetenv here can crash Emacs.
+	 See Bug#8705.  */
+      tzval[0] = 't';
+      tzval[tzeqlen] = 0;
     }
-  tzset ();
-  tzvalbuf_in_environ = 0;
-#endif
 
-  if (!tzstring)
-    {
-      unsetenv ("TZ");
-      tzvalbuf_in_environ = 0;
-    }
-  else
+  if (new_tzvalbuf)
     {
-      ptrdiff_t tzstringlen = strlen (tzstring);
-
-      if (tzvalbufsize <= tzeqlen + tzstringlen)
-	{
-	  unsetenv ("TZ");
-	  tzvalbuf_in_environ = 0;
-	  tzvalbuf = xpalloc (tzvalbuf, &tzvalbufsize,
-			      tzeqlen + tzstringlen - tzvalbufsize + 1, -1, 1);
-	  memcpy (tzvalbuf, "TZ=", tzeqlen);
-	}
-
-      strcpy (tzvalbuf + tzeqlen, tzstring);
-
-      if (!tzvalbuf_in_environ)
-	{
-	  xputenv (tzvalbuf);
-	  tzvalbuf_in_environ = 1;
-	}
+      /* Although this is not thread-safe, in practice this runs only
+	 on startup when there is only one thread.  */
+      xputenv (tzval);
     }
 
-#ifdef LOCALTIME_CACHE
+#ifdef HAVE_TZSET
   tzset ();
 #endif
 }