Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <488f8d57-555b-9328-f768-5779ea8aa8ec@gmail.com>
Date: Fri, 9 Jun 2017 17:15:40 +1000
From: Patrick Oppenlander <pattyo.lists@...il.com>
To: musl@...ts.openwall.com
Subject: detect timezone changes by monitoring /etc/localtime (like glibc)

During some recent testing I came across a bug when adjusting timezones on an
embedded system by changing /etc/localtime. The cause ended up being a
behavioural difference between glibc and musl.

A program compiled against musl will not detect changes to /etc/localtime,
while a program compiled against glibc will.

Intuitively it seems like correct behaviour to me to detect and act upon system
timezone changes, however I am not familiar with any pertinent specifications
for this.

Under glibc if you set TZ=:/etc/localtime changes will not be detected and no
extra syscalls are invoked. This is equivalent to the current musl behaviour.

Given the above I have attached a proposed patch for review. This is
compile-tested only as I ran out of time and would like reviewer comments
before going any further. Is a change in this direction acceptable?

One possible (unlikely?) problem could be if TZ changes in such a way as to
match the cached stat values.

		Patrick

===8<===

diff --git a/src/time/__tz.c b/src/time/__tz.c
index ffe8d402..305616d5 100644
--- a/src/time/__tz.c
+++ b/src/time/__tz.c
@@ -3,6 +3,7 @@
  #include <limits.h>
  #include <stdlib.h>
  #include <string.h>
+#include <sys/stat.h>
  #include "libc.h"
  
  long  __timezone = 0;
@@ -17,15 +18,24 @@ static char std_name[TZNAME_MAX+1];
  static char dst_name[TZNAME_MAX+1];
  const char __gmt[] = "GMT";
  
+static const char etc_lt[] = "/etc/localtime";
+
  static int dst_off;
  static int r0[5], r1[5];
  
  static const unsigned char *zi, *trans, *index, *types, *abbrevs, *abbrevs_end;
  static size_t map_size;
  
-static char old_tz_buf[32];
-static char *old_tz = old_tz_buf;
-static size_t old_tz_size = sizeof old_tz_buf;
+static union {
+	char tz_buf[32];
+	struct {
+		ino_t ino;
+		dev_t dev;
+		time_t mtime;
+	};
+} old;
+static char *old_tz = old.tz_buf;
+static size_t old_tz_size = sizeof old.tz_buf;
  
  static volatile int lock[2];
  
@@ -123,27 +133,37 @@ static void do_tzset()
  	size_t i;
  	static const char search[] =
  		"/usr/share/zoneinfo/\0/share/zoneinfo/\0/etc/zoneinfo/\0";
+	struct stat st;
  
  	s = getenv("TZ");
-	if (!s) s = "/etc/localtime";
+	if (!s) s = stat(etc_lt, &st) == 0 ? etc_lt : __gmt;
  	if (!*s) s = __gmt;
  
-	if (old_tz && !strcmp(s, old_tz)) return;
+	if (s == etc_lt && st.st_ino == old.ino && st.st_dev == old.dev &&
+	    st.st_mtime == old.mtime) return;
+	if (s != etc_lt && old_tz && !strcmp(s, old_tz)) return;
  
  	if (zi) __munmap((void *)zi, map_size);
  
-	/* Cache the old value of TZ to check if it has changed. Avoid
-	 * free so as not to pull it into static programs. Growth
-	 * strategy makes it so free would have minimal benefit anyway. */
-	i = strlen(s);
-	if (i > PATH_MAX+1) s = __gmt, i = 3;
-	if (i >= old_tz_size) {
-		old_tz_size *= 2;
-		if (i >= old_tz_size) old_tz_size = i+1;
-		if (old_tz_size > PATH_MAX+2) old_tz_size = PATH_MAX+2;
-		old_tz = malloc(old_tz_size);
+	/* Cache the old value of TZ or stat of /etc/localtime to check
+	 * if it has changed. Avoid free so as not to pull it into static
+	 * programs. Growth strategy makes it so free would have minimal
+	 * benefit anyway. */
+	if (s == etc_lt) {
+		old.ino = st.st_ino;
+		old.dev = st.st_dev;
+		old.mtime = st.st_mtime;
+	} else {
+		i = strlen(s);
+		if (i > PATH_MAX+1) s = __gmt, i = 3;
+		if (i >= old_tz_size) {
+			old_tz_size *= 2;
+			if (i >= old_tz_size) old_tz_size = i+1;
+			if (old_tz_size > PATH_MAX+2) old_tz_size = PATH_MAX+2;
+			old_tz = malloc(old_tz_size);
+		}
+		if (old_tz) memcpy(old_tz, s, i+1);
  	}
-	if (old_tz) memcpy(old_tz, s, i+1);
  
  	/* Non-suid can use an absolute tzfile pathname or a relative
  	 * pathame beginning with "."; in secure mode, only the
@@ -151,7 +171,7 @@ static void do_tzset()
  	if (*s == ':' || ((p=strchr(s, '/')) && !memchr(s, ',', p-s))) {
  		if (*s == ':') s++;
  		if (*s == '/' || *s == '.') {
-			if (!libc.secure || !strcmp(s, "/etc/localtime"))
+			if (!libc.secure || !strcmp(s, etc_lt))
  				map = __map_file(s, &map_size);
  		} else {
  			size_t l = strlen(s);

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.