Merge "Don't use so much stack in tzcode."

libc/Android.mk

@@ -511,12 +511,19 @@ include $(BUILD_STATIC_LIBRARY)
 include $(CLEAR_VARS)
 
 LOCAL_SRC_FILES := $(call all-c-files-under,tzcode)
-LOCAL_CFLAGS := \
+
-    $(libc_common_cflags) \
+LOCAL_CFLAGS := $(libc_common_cflags)
-    -DSTD_INSPIRED=1 \
+# Don't use ridiculous amounts of stack.
-    -DTZDIR=\"/system/usr/share/zoneinfo\" \
+LOCAL_CFLAGS += -DALL_STATE
-    -DTM_GMTOFF=tm_gmtoff \
+# Include tzsetwall, timelocal, timegm, time2posix, and posix2time.
-    -DUSG_COMPAT=1
+LOCAL_CFLAGS += -DSTD_INSPIRED
+# The name of the tm_gmtoff field in our struct tm.
+LOCAL_CFLAGS += -DTM_GMTOFF=tm_gmtoff
+# Where we store our tzdata.
+LOCAL_CFLAGS += -DTZDIR=\"/system/usr/share/zoneinfo\"
+# Include timezone and daylight globals.
+LOCAL_CFLAGS += -DUSG_COMPAT=1
+
 LOCAL_CONLYFLAGS := $(libc_common_conlyflags)
 LOCAL_CPPFLAGS := $(libc_common_cppflags)
 LOCAL_C_INCLUDES := $(libc_common_c_includes)

libc/tzcode/localtime.c

@@ -2115,11 +2115,17 @@ static int __bionic_open_tzdata_path(const char* path_prefix_variable, const cha
     fprintf(stderr, "%s: %s not set!\n", __FUNCTION__, path_prefix_variable);
     return -1;
   }
-  char path[PATH_MAX];
+  size_t path_length = strlen(path_prefix) + 1 + strlen(path_suffix) + 1;
-  snprintf(path, sizeof(path), "%s/%s", path_prefix, path_suffix);
+  char* path = malloc(path_length);
+  if (path == NULL) {
+    fprintf(stderr, "%s: couldn't allocate %zu-byte path\n", __FUNCTION__, path_length);
+    return -1;
+  }
+  snprintf(path, path_length, "%s/%s", path_prefix, path_suffix);
   int fd = TEMP_FAILURE_RETRY(open(path, OPEN_MODE));
   if (fd == -1) {
     XLOG(("%s: could not open \"%s\": %s\n", __FUNCTION__, path, strerror(errno)));
+    free(path);
     return -2; // Distinguish failure to find any data from failure to find a specific id.
   }
 
@@ -2138,6 +2144,7 @@ static int __bionic_open_tzdata_path(const char* path_prefix_variable, const cha
   if (bytes_read != sizeof(header)) {
     fprintf(stderr, "%s: could not read header of \"%s\": %s\n",
             __FUNCTION__, path, (bytes_read == -1) ? strerror(errno) : "short read");
+    free(path);
     close(fd);
     return -1;
   }
@@ -2145,6 +2152,7 @@ static int __bionic_open_tzdata_path(const char* path_prefix_variable, const cha
   if (strncmp(header.tzdata_version, "tzdata", 6) != 0 || header.tzdata_version[11] != 0) {
     fprintf(stderr, "%s: bad magic in \"%s\": \"%.6s\"\n",
             __FUNCTION__, path, header.tzdata_version);
+    free(path);
     close(fd);
     return -1;
   }
@@ -2159,6 +2167,7 @@ static int __bionic_open_tzdata_path(const char* path_prefix_variable, const cha
   if (TEMP_FAILURE_RETRY(lseek(fd, ntohl(header.index_offset), SEEK_SET)) == -1) {
     fprintf(stderr, "%s: couldn't seek to index in \"%s\": %s\n",
             __FUNCTION__, path, strerror(errno));
+    free(path);
     close(fd);
     return -1;
   }
@@ -2166,9 +2175,17 @@ static int __bionic_open_tzdata_path(const char* path_prefix_variable, const cha
   off_t specific_zone_offset = -1;
   ssize_t index_size = ntohl(header.data_offset) - ntohl(header.index_offset);
   char* index = malloc(index_size);
+  if (index == NULL) {
+    fprintf(stderr, "%s: couldn't allocate %zd-byte index for \"%s\"\n",
+            __FUNCTION__, index_size, path);
+    free(path);
+    close(fd);
+    return -1;
+  }
   if (TEMP_FAILURE_RETRY(read(fd, index, index_size)) != index_size) {
     fprintf(stderr, "%s: could not read index of \"%s\": %s\n",
             __FUNCTION__, path, (bytes_read == -1) ? strerror(errno) : "short read");
+    free(path);
     free(index);
     close(fd);
     return -1;
@@ -2201,6 +2218,7 @@ static int __bionic_open_tzdata_path(const char* path_prefix_variable, const cha
 
   if (specific_zone_offset == -1) {
     XLOG(("%s: couldn't find zone \"%s\"\n", __FUNCTION__, olson_id));
+    free(path);
     close(fd);
     return -1;
   }
@@ -2208,12 +2226,14 @@ static int __bionic_open_tzdata_path(const char* path_prefix_variable, const cha
   if (TEMP_FAILURE_RETRY(lseek(fd, specific_zone_offset, SEEK_SET)) == -1) {
     fprintf(stderr, "%s: could not seek to %ld in \"%s\": %s\n",
             __FUNCTION__, specific_zone_offset, path, strerror(errno));
+    free(path);
     close(fd);
     return -1;
   }
 
   // TODO: check that there's TZ_MAGIC at this offset, so we can fall back to the other file if not.
 
+  free(path);
   return fd;
 }
 

tests/time_test.cpp

@@ -19,6 +19,7 @@
 #include <errno.h>
 #include <features.h>
 #include <gtest/gtest.h>
+#include <pthread.h>
 #include <signal.h>
 #include <sys/types.h>
 #include <sys/wait.h>
@@ -65,6 +66,37 @@ TEST(time, gmtime) {
   ASSERT_EQ(1970, broken_down->tm_year + 1900);
 }
 
+static void* gmtime_no_stack_overflow_14313703_fn(void*) {
+  const char* original_tz = getenv("TZ");
+  // Ensure we'll actually have to enter tzload by using a time zone that doesn't exist.
+  setenv("TZ", "gmtime_stack_overflow_14313703", 1);
+  tzset();
+  if (original_tz != NULL) {
+    setenv("TZ", original_tz, 1);
+  }
+  tzset();
+  return NULL;
+}
+
+TEST(time, gmtime_no_stack_overflow_14313703) {
+  // Is it safe to call tzload on a thread with a small stack?
+  // http://b/14313703
+  // https://code.google.com/p/android/issues/detail?id=61130
+  pthread_attr_t attributes;
+  ASSERT_EQ(0, pthread_attr_init(&attributes));
+#if defined(__BIONIC__)
+  ASSERT_EQ(0, pthread_attr_setstacksize(&attributes, PTHREAD_STACK_MIN));
+#else
+  // PTHREAD_STACK_MIN not currently in the host GCC sysroot.
+  ASSERT_EQ(0, pthread_attr_setstacksize(&attributes, 4 * getpagesize()));
+#endif
+
+  pthread_t t;
+  ASSERT_EQ(0, pthread_create(&t, &attributes, gmtime_no_stack_overflow_14313703_fn, NULL));
+  void* result;
+  ASSERT_EQ(0, pthread_join(t, &result));
+}
+
 TEST(time, mktime_10310929) {
   struct tm t;
   memset(&t, 0, sizeof(tm));