[Emacs-diffs] /srv/bzr/emacs/trunk r108109: Fix race conditions involving setenv, gmtime, localtime, asctime.

[Paul Eggert]

------------------------------------------------------------
revno: 108109
committer: Paul Eggert <address@hidden>
branch nick: trunk
timestamp: Wed 2012-05-02 16:25:46 -0700
message:
  Fix race conditions involving setenv, gmtime, localtime, asctime.
  
  Without this fix, interrupts could mess up code that uses these
  nonreentrant functions, since setting TZ invalidates existing
  tm_zone or tzname values, and since most of these functions return
  pointers to static storage.
  * editfns.c (format_time_string, Fdecode_time, Fencode_time)
  (Fcurrent_time_string, Fcurrent_time_zone, Fset_time_zone_rule):
  Grow the critical sections to include not just invoking
  localtime/gmtime, but also accessing these functions' results
  including their tm_zone values if any, and any related TZ setting.
  (format_time_string): Last arg is now struct tm *, not struct tm **,
  so that the struct tm is saved in the critical section.  All
  callers changed.  Simplify allocation of initial buffer, partly
  motivated by the fact that memory allocation needs to be outside
  the critical section.
modified:
  src/ChangeLog
  src/editfns.c

=== modified file 'src/ChangeLog'
--- a/src/ChangeLog     2012-05-02 17:21:54 +0000
+++ b/src/ChangeLog     2012-05-02 23:25:46 +0000
@@ -1,3 +1,21 @@
+2012-05-02  Paul Eggert  <address@hidden>
+
+       Fix race conditions involving setenv, gmtime, localtime, asctime.
+       Without this fix, interrupts could mess up code that uses these
+       nonreentrant functions, since setting TZ invalidates existing
+       tm_zone or tzname values, and since most of these functions return
+       pointers to static storage.
+       * editfns.c (format_time_string, Fdecode_time, Fencode_time)
+       (Fcurrent_time_string, Fcurrent_time_zone, Fset_time_zone_rule):
+       Grow the critical sections to include not just invoking
+       localtime/gmtime, but also accessing these functions' results
+       including their tm_zone values if any, and any related TZ setting.
+       (format_time_string): Last arg is now struct tm *, not struct tm **,
+       so that the struct tm is saved in the critical section.  All
+       callers changed.  Simplify allocation of initial buffer, partly
+       motivated by the fact that memory allocation needs to be outside
+       the critical section.
+
 2012-05-02  Dmitry Antipov  <address@hidden>
 
        * intervals.c (adjust_intervals_for_insertion): Initialize `newi'

=== modified file 'src/editfns.c'
--- a/src/editfns.c     2012-03-11 16:27:36 +0000
+++ b/src/editfns.c     2012-05-02 23:25:46 +0000
@@ -86,7 +86,7 @@
 
 static void time_overflow (void) NO_RETURN;
 static Lisp_Object format_time_string (char const *, ptrdiff_t, Lisp_Object,
-                                      int, time_t *, struct tm **);
+                                      int, time_t *, struct tm *);
 static int tm_diff (struct tm *, struct tm *);
 static void update_buffer_properties (EMACS_INT, EMACS_INT);
 
@@ -1704,7 +1704,7 @@
   (Lisp_Object format_string, Lisp_Object timeval, Lisp_Object universal)
 {
   time_t t;
-  struct tm *tm;
+  struct tm tm;
 
   CHECK_STRING (format_string);
   format_string = code_convert_string_norecord (format_string,
@@ -1715,54 +1715,55 @@
 
 static Lisp_Object
 format_time_string (char const *format, ptrdiff_t formatlen,
-                   Lisp_Object timeval, int ut, time_t *tval, struct tm **tmp)
+                   Lisp_Object timeval, int ut, time_t *tval, struct tm *tmp)
 {
-  ptrdiff_t size;
+  char buffer[4000];
+  char *buf = buffer;
+  size_t size = sizeof buffer;
+  size_t len;
+  Lisp_Object bufstring;
   int usec;
   int ns;
   struct tm *tm;
+  USE_SAFE_ALLOCA;
 
   if (! (lisp_time_argument (timeval, tval, &usec)
         && 0 <= usec && usec < 1000000))
     error ("Invalid time specification");
   ns = usec * 1000;
 
-  /* This is probably enough.  */
-  size = formatlen;
-  if (size <= (STRING_BYTES_BOUND - 50) / 6)
-    size = size * 6 + 50;
-
-  BLOCK_INPUT;
-  tm = ut ? gmtime (tval) : localtime (tval);
-  UNBLOCK_INPUT;
-  if (! tm)
-    time_overflow ();
-  *tmp = tm;
-
-  synchronize_system_time_locale ();
-
   while (1)
     {
-      char *buf = (char *) alloca (size + 1);
-      size_t result;
+      BLOCK_INPUT;
+
+      synchronize_system_time_locale ();
+
+      tm = ut ? gmtime (tval) : localtime (tval);
+      if (! tm)
+       {
+         UNBLOCK_INPUT;
+         time_overflow ();
+       }
+      *tmp = *tm;
 
       buf[0] = '\1';
-      BLOCK_INPUT;
-      result = emacs_nmemftime (buf, size, format, formatlen, tm, ut, ns);
-      UNBLOCK_INPUT;
-      if ((result > 0 && result < size) || (result == 0 && buf[0] == '\0'))
-       return code_convert_string_norecord (make_unibyte_string (buf, result),
-                                            Vlocale_coding_system, 0);
+      len = emacs_nmemftime (buf, size, format, formatlen, tm, ut, ns);
+      if ((0 < len && len < size) || (len == 0 && buf[0] == '\0'))
+       break;
 
-      /* If buffer was too small, make it bigger and try again.  */
-      BLOCK_INPUT;
-      result = emacs_nmemftime (NULL, (size_t) -1, format, formatlen,
-                               tm, ut, ns);
+      /* Buffer was too small, so make it bigger and try again.  */
+      len = emacs_nmemftime (NULL, SIZE_MAX, format, formatlen, tm, ut, ns);
       UNBLOCK_INPUT;
-      if (STRING_BYTES_BOUND <= result)
+      if (STRING_BYTES_BOUND <= len)
        string_overflow ();
-      size = result + 1;
+      size = len + 1;
+      SAFE_ALLOCA (buf, char *, size);
     }
+
+  UNBLOCK_INPUT;
+  bufstring = make_unibyte_string (buf, len);
+  SAFE_FREE ();
+  return code_convert_string_norecord (bufstring, Vlocale_coding_system, 0);
 }
 
 DEFUN ("decode-time", Fdecode_time, Sdecode_time, 0, 1, 0,
@@ -1792,31 +1793,32 @@
 
   BLOCK_INPUT;
   decoded_time = localtime (&time_spec);
+  /* Make a copy, in case a signal handler modifies TZ or the struct.  */
+  if (decoded_time)
+    save_tm = *decoded_time;
   UNBLOCK_INPUT;
   if (! (decoded_time
-        && MOST_NEGATIVE_FIXNUM - TM_YEAR_BASE <= decoded_time->tm_year
-        && decoded_time->tm_year <= MOST_POSITIVE_FIXNUM - TM_YEAR_BASE))
+        && MOST_NEGATIVE_FIXNUM - TM_YEAR_BASE <= save_tm.tm_year
+        && save_tm.tm_year <= MOST_POSITIVE_FIXNUM - TM_YEAR_BASE))
     time_overflow ();
-  XSETFASTINT (list_args[0], decoded_time->tm_sec);
-  XSETFASTINT (list_args[1], decoded_time->tm_min);
-  XSETFASTINT (list_args[2], decoded_time->tm_hour);
-  XSETFASTINT (list_args[3], decoded_time->tm_mday);
-  XSETFASTINT (list_args[4], decoded_time->tm_mon + 1);
+  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) decoded_time->tm_year);
-  XSETFASTINT (list_args[6], decoded_time->tm_wday);
-  list_args[7] = (decoded_time->tm_isdst)? Qt : Qnil;
+  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;
 
-  /* Make a copy, in case gmtime modifies the struct.  */
-  save_tm = *decoded_time;
   BLOCK_INPUT;
   decoded_time = gmtime (&time_spec);
-  UNBLOCK_INPUT;
   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);
 }
 
@@ -1898,21 +1900,23 @@
       else
        error ("Invalid time zone specification");
 
+      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);
 
-      BLOCK_INPUT;
       value = mktime (&tm);
-      UNBLOCK_INPUT;
 
       /* Restore TZ to previous value.  */
       newenv = environ;
       environ = oldenv;
+#ifdef LOCALTIME_CACHE
+      tzset ();
+#endif
+      UNBLOCK_INPUT;
+
       xfree (newenv);
-#ifdef LOCALTIME_CACHE
-      tzset ();
-#endif
     }
 
   if (value == (time_t) -1)
@@ -1939,24 +1943,29 @@
 {
   time_t value;
   struct tm *tm;
-  register char *tem;
+  char *tem = NULL;
+  char buf[sizeof "Mon Apr 30 12:49:17 2012" - 1];
 
   if (! lisp_time_argument (specified_time, &value, NULL))
     error ("Invalid time specification");
 
   /* Convert to a string, checking for out-of-range time stamps.
+     Omit the trailing newline.
      Don't use 'ctime', as that might dump core if VALUE is out of
      range.  */
   BLOCK_INPUT;
   tm = localtime (&value);
+  if (tm && TM_YEAR_IN_ASCTIME_RANGE (tm->tm_year))
+    {
+      tem = asctime (tm);
+      if (tem)
+       memcpy (buf, tem, sizeof buf);
+    }
   UNBLOCK_INPUT;
-  if (! (tm && TM_YEAR_IN_ASCTIME_RANGE (tm->tm_year) && (tem = asctime (tm))))
+  if (! tem)
     time_overflow ();
 
-  /* Remove the trailing newline.  */
-  tem[strlen (tem) - 1] = '\0';
-
-  return build_string (tem);
+  return make_unibyte_string (buf, sizeof buf);
 }
 
 /* Yield A - B, measured in seconds.
@@ -2000,22 +2009,22 @@
   (Lisp_Object specified_time)
 {
   time_t value;
+  int offset;
   struct tm *t;
   struct tm localtm;
-  struct tm *localt;
   Lisp_Object zone_offset, zone_name;
 
   zone_offset = Qnil;
   zone_name = format_time_string ("%Z", sizeof "%Z" - 1, specified_time,
-                                 0, &value, &localt);
-  localtm = *localt;
+                                 0, &value, &localtm);
   BLOCK_INPUT;
   t = gmtime (&value);
+  if (t)
+    offset = tm_diff (&localtm, t);
   UNBLOCK_INPUT;
 
   if (t)
     {
-      int offset = tm_diff (&localtm, t);
       zone_offset = make_number (offset);
       if (SCHARS (zone_name) == 0)
        {
@@ -2053,9 +2062,16 @@
   (Lisp_Object tz)
 {
   const char *tzstring;
+  char **old_environbuf;
+
+  if (! (NILP (tz) || EQ (tz, Qt)))
+    CHECK_STRING (tz);
+
+  BLOCK_INPUT;
 
   /* When called for the first time, save the original TZ.  */
-  if (!environbuf)
+  old_environbuf = environbuf;
+  if (!old_environbuf)
     initial_tz = (char *) getenv ("TZ");
 
   if (NILP (tz))
@@ -2063,15 +2079,14 @@
   else if (EQ (tz, Qt))
     tzstring = "UTC0";
   else
-    {
-      CHECK_STRING (tz);
-      tzstring = SSDATA (tz);
-    }
+    tzstring = SSDATA (tz);
 
   set_time_zone_rule (tzstring);
-  xfree (environbuf);
   environbuf = environ;
 
+  UNBLOCK_INPUT;
+
+  xfree (old_environbuf);
   return Qnil;
 }