Merge branch 'bug8787_squashed'

4 files changed, 93 insertions, 20 deletions

diff --git a/src/common/compat.c b/src/common/compat.c
index 8e2619f846..135f2c9af6 100644
--- a/src/common/compat.c
+++ b/src/common/compat.c
@@ -35,6 +35,12 @@
 #ifdef HAVE_UNAME
 #include <sys/utsname.h>
 #endif
+#ifdef HAVE_SYS_TYPES_H
+#include <sys/types.h>
+#endif
+#ifdef HAVE_SYS_STAT_H
+#include <sys/stat.h>
+#endif
 #ifdef HAVE_UNISTD_H
 #include <unistd.h>
 #endif
@@ -178,9 +184,10 @@ tor_mmap_file(const char *filename)
 {
   int fd; /* router file */
   char *string;
-  int page_size;
+  int page_size, result;
   tor_mmap_t *res;
   size_t size, filesize;
+  struct stat st;
 
   tor_assert(filename);
 
@@ -194,9 +201,22 @@ tor_mmap_file(const char *filename)
     return NULL;
   }
 
-  /* XXXX why not just do fstat here? */
-  size = filesize = (size_t) lseek(fd, 0, SEEK_END);
-  lseek(fd, 0, SEEK_SET);
+  /* Get the size of the file */
+  result = fstat(fd, &st);
+  if (result != 0) {
+    int save_errno = errno;
+    log_warn(LD_FS,
+             "Couldn't fstat opened descriptor for \"%s\" during mmap: %s",
+             filename, strerror(errno));
+    close(fd);
+    errno = save_errno;
+    return NULL;
+  }
+  size = filesize = (size_t)(st.st_size);
+  /*
+   * Should we check for weird crap like mmapping a named pipe here,
+   * or just wait for if (!size) below to fail?
+   */
   /* ensure page alignment */
   page_size = getpagesize();
   size += (size%page_size) ? page_size-(size%page_size) : 0;
@@ -227,12 +247,27 @@ tor_mmap_file(const char *filename)
 
   return res;
 }
-/** Release storage held for a memory mapping. */
-void
+/** Release storage held for a memory mapping; returns 0 on success,
+ * or -1 on failure (and logs a warning). */
+int
 tor_munmap_file(tor_mmap_t *handle)
 {
-  munmap((char*)handle->data, handle->mapping_size);
-  tor_free(handle);
+  int res;
+
+  if (handle == NULL)
+    return 0;
+
+  res = munmap((char*)handle->data, handle->mapping_size);
+  if (res == 0) {
+    /* munmap() succeeded */
+    tor_free(handle);
+  } else {
+    log_warn(LD_FS, "Failed to munmap() in tor_munmap_file(): %s",
+             strerror(errno));
+    res = -1;
+  }
+
+  return res;
 }
 #elif defined(_WIN32)
 tor_mmap_t *
@@ -314,17 +349,29 @@ tor_mmap_file(const char *filename)
   tor_munmap_file(res);
   return NULL;
 }
-void
+
+/* Unmap the file, and return 0 for success or -1 for failure */
+int
 tor_munmap_file(tor_mmap_t *handle)
 {
-  if (handle->data)
+  if (handle == NULL)
+    return 0;
+
+  if (handle->data) {
     /* This is an ugly cast, but without it, "data" in struct tor_mmap_t would
        have to be redefined as non-const. */
-    UnmapViewOfFile( (LPVOID) handle->data);
+    BOOL ok = UnmapViewOfFile( (LPVOID) handle->data);
+    if (!ok) {
+      log_warn(LD_FS, "Failed to UnmapViewOfFile() in tor_munmap_file(): %d",
+               (int)GetLastError());
+    }
+  }
 
   if (handle->mmap_handle != NULL)
     CloseHandle(handle->mmap_handle);
   tor_free(handle);
+
+  return 0;
 }
 #else
 tor_mmap_t *
@@ -340,13 +387,25 @@ tor_mmap_file(const char *filename)
   handle->size = st.st_size;
   return handle;
 }
-void
+
+/** Unmap the file mapped with tor_mmap_file(), and return 0 for success
+ * or -1 for failure.
+ */
+
+int
 tor_munmap_file(tor_mmap_t *handle)
 {
-  char *d = (char*)handle->data;
+  char *d = NULL;
+  if (handle == NULL)
+    return 0;
+
+  d = (char*)handle->data;
   tor_free(d);
   memwipe(handle, 0, sizeof(tor_mmap_t));
   tor_free(handle);
+
+  /* Can't fail in this mmap()/munmap()-free case */
+  return 0;
 }
 #endif
 
diff --git a/src/common/compat.h b/src/common/compat.h
index 32effa5c74..c46446414f 100644
--- a/src/common/compat.h
+++ b/src/common/compat.h
@@ -292,7 +292,7 @@ typedef struct tor_mmap_t {
 } tor_mmap_t;
 
 tor_mmap_t *tor_mmap_file(const char *filename) ATTR_NONNULL((1));
-void tor_munmap_file(tor_mmap_t *handle) ATTR_NONNULL((1));
+int tor_munmap_file(tor_mmap_t *handle) ATTR_NONNULL((1));
 
 int tor_snprintf(char *str, size_t size, const char *format, ...)
   CHECK_PRINTF(3,4) ATTR_NONNULL((1,3));
diff --git a/src/common/tortls.c b/src/common/tortls.c
index 8defab2adb..9ba8fd683e 100644
--- a/src/common/tortls.c
+++ b/src/common/tortls.c
@@ -2312,6 +2312,7 @@ log_cert_lifetime(int severity, const X509 *cert, const char *problem)
   char mytime[33];
   time_t now = time(NULL);
   struct tm tm;
+  size_t n;
 
   if (problem)
     tor_log(severity, LD_GENERAL,
@@ -2337,11 +2338,17 @@ log_cert_lifetime(int severity, const X509 *cert, const char *problem)
   BIO_get_mem_ptr(bio, &buf);
   s2 = tor_strndup(buf->data, buf->length);
 
-  strftime(mytime, 32, "%b %d %H:%M:%S %Y UTC", tor_gmtime_r(&now, &tm));
-
-  tor_log(severity, LD_GENERAL,
-      "(certificate lifetime runs from %s through %s. Your time is %s.)",
-      s1,s2,mytime);
+  n = strftime(mytime, 32, "%b %d %H:%M:%S %Y UTC", tor_gmtime_r(&now, &tm));
+  if (n > 0) {
+    tor_log(severity, LD_GENERAL,
+        "(certificate lifetime runs from %s through %s. Your time is %s.)",
+        s1,s2,mytime);
+  } else {
+    tor_log(severity, LD_GENERAL,
+        "(certificate lifetime runs from %s through %s. "
+        "Couldn't get your time.)",
+        s1, s2);
+  }
 
  end:
   /* Not expected to get invoked */
diff --git a/src/common/util.c b/src/common/util.c
index a0adb15ffe..a50f2566db 100644
--- a/src/common/util.c
+++ b/src/common/util.c
@@ -2141,6 +2141,7 @@ static int
 finish_writing_to_file_impl(open_file_t *file_data, int abort_write)
 {
   int r = 0;
+
   tor_assert(file_data && file_data->filename);
   if (file_data->stdio_file) {
     if (fclose(file_data->stdio_file)) {
@@ -2157,7 +2158,13 @@ finish_writing_to_file_impl(open_file_t *file_data, int abort_write)
   if (file_data->rename_on_close) {
     tor_assert(file_data->tempname && file_data->filename);
     if (abort_write) {
-      unlink(file_data->tempname);
+      int res = unlink(file_data->tempname);
+      if (res != 0) {
+        /* We couldn't unlink and we'll leave a mess behind */
+        log_warn(LD_FS, "Failed to unlink %s: %s",
+                 file_data->tempname, strerror(errno));
+        r = -1;
+      }
     } else {
       tor_assert(strcmp(file_data->filename, file_data->tempname));
       if (replace_file(file_data->tempname, file_data->filename)) {