detect timezone changes by monitoring /etc/localtime (like glibc)

detect timezone changes by monitoring /etc/localtime (like glibc)

[Patrick Oppenlander]

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);

Re: detect timezone changes by monitoring /etc/localtime (like glibc)

[Rich Felker]

On Fri, Jun 09, 2017 at 05:15:40PM +1000, Patrick Oppenlander wrote:
> 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.

This difference is intentional; I believe there are past discussions
in the list archives.

Aside from the glibc behavior giving abysmal performance (syscalls on
every time operation), there's a more fundamental issue of usability
of the results. A common important idiom with time functions is to
perform several operations together in succession to get a result --
for example, obtain the current time as time_t, format it with
localtime[_r], make some adjustment e.g. for a relative time, then
call mktime to convert back to time_t. Such operations rely on the
time zone being consistent for each suboperation, which is true as
long as the application does not modify its own environment, but only
assuming libc doesn't impose asynchronous changes on the timezone.

In order for asynchronous timezone changes to be safe, the API would
have to be such that you make one call to get the current timezone,
then pass it as an argument to functions depending on the timezone, so
that the application has control of which timezone is getting used in
each call.

Rich

Re: detect timezone changes by monitoring /etc/localtime (like glibc)

[A. Wilcox]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

On 09/06/17 07:15, Rich Felker wrote:
> On Fri, Jun 09, 2017 at 05:15:40PM +1000, Patrick Oppenlander
> wrote:
>> 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.
> 
> This difference is intentional; I believe there are past
> discussions in the list archives.
> 
> Aside from the glibc behavior giving abysmal performance (syscalls
> on every time operation), there's a more fundamental issue of
> usability of the results. A common important idiom with time
> functions is to perform several operations together in succession
> to get a result -- for example, obtain the current time as time_t,
> format it with localtime[_r], make some adjustment e.g. for a
> relative time, then call mktime to convert back to time_t. Such
> operations rely on the time zone being consistent for each
> suboperation, which is true as long as the application does not
> modify its own environment, but only assuming libc doesn't impose
> asynchronous changes on the timezone.


Fun and true facts: this has actually revealed itself as a horrible
behaviour in Pidgin.  When I moved from the east coast to the central
US, I woke up my laptop and changed the TZ.  It was running a glibc
Linux at that point.  Pidgin wouldn't make noise or show notifications
until an hour went by; some internal counter had a local time_t and it
thought it was receiving old notifications (since the timezone shifted
backwards an hour).

So this isn't a theoretical problem.  It's a very real one.  (And one
I have yet to experience running musl Linux on my laptop now.)

- --arw

- -- 
A. Wilcox (awilfox)
Project Lead, Adélie Linux
http://adelielinux.org
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAEBCAAGBQJZO2k6AAoJEMspy1GSK50UcqoP/1+W4LHdIqHjiPLigvKJSuRG
nr1DfpsFDyOxPYZa7fJMvkUrycsgM8o9kWlIuoGss8eqGQa6iW95hk4dVmnzjTOF
YELSQYxZ0X0w2dFYC2i5WwLs5MuG2gjg1Egr70i5i2uFBkm2qkl5qtmX3hGdU6V/
yPtOOaAGvgLgrKxjq5BjlssGVgqhLtPD4Y3cT1HAg6UZMtKHN3hcufeRYbha12VE
UW90k/XcoOOTvZCAAKZrbCbqzAjtuplIOB6uaTjNUpDmlhHXHy+uE4GJOQ3ohdWV
9occL5wGNsj+BM6+4qhhSmBTJdbHxwZ32t+0XWTejE0imDB1Y+zFphGX9utN6PwM
Fkk6BcJTK8tg1/gbXwaHNYNe6L9hnnynnLo2tpWI64zZAgDDhUpkWx4sojAA795q
9AgjcbWucXxFkTA3gZfouWSBWpuOhUBPvvoFo0oIxbhTSzgCSEw+0C7WaVcPlPrh
o7osd8WLm2U7ektOXgVSfOYBTMKYp49DekgVb4xVhSteA0L2p7BzZIxvPc0ebf7k
jHPplp1WcDKJ/wJ6nybVxwRCrHDFa1lZZ62vdotKuzeNcUG2HPPlx68RA0Qsmtzw
UP1yeUefOpoW1wq+jhBcV2HwLRyAZFLYqfuF/zyZ9cCBOBtC1U4uzbN0OBdAdOUX
6kwppw1yXr1xZQudzKjU
=QbW2
-----END PGP SIGNATURE-----

Re: detect timezone changes by monitoring /etc/localtime (like glibc)

[u-uy74]

On Fri, Jun 09, 2017 at 10:36:29PM -0500, A. Wilcox wrote:
> Fun and true facts: this has actually revealed itself as a horrible
> behaviour in Pidgin.  When I moved from the east coast to the central
> US, I woke up my laptop and changed the TZ.  It was running a glibc
> Linux at that point.  Pidgin wouldn't make noise or show notifications
> until an hour went by; some internal counter had a local time_t and it
> thought it was receiving old notifications (since the timezone shifted
> backwards an hour).

To me it looks like an application [design] bug. Time itself does
not shift because you are moving to a different angle from the sun,
so deciding "new" vs "old" should not be concerned with timezones or
"local time" at all.

It is the _presentation to humans_ which may need to show
the "local time" but there is no reason to use it for timekeeping.
Moreover, there are good reasons to avoid such use.

> So this isn't a theoretical problem.  It's a very real one.  (And one
> I have yet to experience running musl Linux on my laptop now.)

Yes, a real problem, but outside libc, imho.

Cheers,
Rune

Re: detect timezone changes by monitoring /etc/localtime (like glibc)

[Rich Felker]

On Sat, Jun 10, 2017 at 11:57:09AM +0200, u-uy74@aetey.se wrote:
> On Fri, Jun 09, 2017 at 10:36:29PM -0500, A. Wilcox wrote:
> > Fun and true facts: this has actually revealed itself as a horrible
> > behaviour in Pidgin.  When I moved from the east coast to the central
> > US, I woke up my laptop and changed the TZ.  It was running a glibc
> > Linux at that point.  Pidgin wouldn't make noise or show notifications
> > until an hour went by; some internal counter had a local time_t and it
> > thought it was receiving old notifications (since the timezone shifted
> > backwards an hour).
> 
> To me it looks like an application [design] bug. Time itself does
> not shift because you are moving to a different angle from the sun,
> so deciding "new" vs "old" should not be concerned with timezones or
> "local time" at all.
> 
> It is the _presentation to humans_ which may need to show
> the "local time" but there is no reason to use it for timekeeping.
> Moreover, there are good reasons to avoid such use.
> 
> > So this isn't a theoretical problem.  It's a very real one.  (And one
> > I have yet to experience running musl Linux on my laptop now.)
> 
> Yes, a real problem, but outside libc, imho.

It's a library issue because, if the invariance of timezone when the
application doesn't intentionally change it is violated on the libc
side, there's no way for applications to meaningfully perform these
kinds of relative operations on local times -- the API is deficient. I
agree the pidgin thing sounds like a pidgin bug (wrongly working with
local times) but there are situations where you need to, or where
you'd prefer to be working in UTC but due to lack of timegm in
standard C, a roundabout route through localtime/mktime and adjustment
for localtime is the only portable solution that's possible.

Rich

Re: detect timezone changes by monitoring /etc/localtime (like glibc)

[u-uy74]

On Sat, Jun 10, 2017 at 08:23:04AM -0400, Rich Felker wrote:
> On Sat, Jun 10, 2017 at 11:57:09AM +0200, u-uy74@aetey.se wrote:
> > 
> > It is the _presentation to humans_ which may need to show
> > the "local time" but there is no reason to use it for timekeeping.
> > Moreover, there are good reasons to avoid such use.

> > ... a real problem, but outside libc, imho.

> It's a library issue because, if the invariance of timezone when the
> application doesn't intentionally change it is violated on the libc
> side, there's no way for applications to meaningfully perform these
> kinds of relative operations on local times -- the API is deficient.

As I see it, the time zone is an application internal business, not a
"computer system" one. Even inside the same running application instance
different presentations may need to account for different time zones,
say if the application serves multiple users concurrently or if it
prepares data for multiple uses going to consume the data (say a report)
in different time zones.

It is not only that the API is badly designed but also the design was
highly misdirected :(

In a sense, an application relying on a "system wide time zone"
can not be expected to work reliably, because there is no well defined
semantics of such a thing. The misperception comes from the time when
computers were stationary and isolated from each other.

> agree the pidgin thing sounds like a pidgin bug (wrongly working with
> local times) but there are situations where you need to, or where
> you'd prefer to be working in UTC but due to lack of timegm in
> standard C, a roundabout route through localtime/mktime and adjustment
> for localtime is the only portable solution that's possible.

Yes I believe I see your point. Still the split form of the time
representation looks to me "human-specific" and I can not easily imagine
when it would make sence to handle this kind of representation outside
of a certain time zone context, the latter targeting humans.

Of course a split internal time representation is a possible application
design choice but it is like designing the microcode of a binary CPU to
do all calculations in (irregularly tweaked) sexagecimal. Possible but
hardly justified beyond some very specific niche cases.

My two cents.

Regards,
Rune

BTW thanks for musl!

Re: detect timezone changes by monitoring /etc/localtime (like glibc)

[Patrick Oppenlander]

On 09/06/17 22:15, Rich Felker wrote:
> On Fri, Jun 09, 2017 at 05:15:40PM +1000, Patrick Oppenlander wrote:
>> 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.
> 
> This difference is intentional; I believe there are past discussions
> in the list archives.
> 

I found https://marc.info/?l=musl&m=141374003126007&w=2 which was helpful.

Performance could be addressed (inotify?) but it's all moot anyway given
the other issues mentioned.

> 
> In order for asynchronous timezone changes to be safe, the API would
> have to be such that you make one call to get the current timezone,
> then pass it as an argument to functions depending on the timezone, so
> that the application has control of which timezone is getting used in
> each call.
> 

This is how the WIN32 API is implemented.

		Patrick

Re: detect timezone changes by monitoring /etc/localtime (like glibc)

[Rich Felker]

On Tue, Jun 13, 2017 at 09:08:15AM +1000, Patrick Oppenlander wrote:
> On 09/06/17 22:15, Rich Felker wrote:
> >On Fri, Jun 09, 2017 at 05:15:40PM +1000, Patrick Oppenlander wrote:
> >>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.
> >
> >This difference is intentional; I believe there are past discussions
> >in the list archives.
> >
> 
> I found https://marc.info/?l=musl&m=141374003126007&w=2 which was helpful.

Nice, I'd forgotten about that.

> Performance could be addressed (inotify?) but it's all moot anyway given
> the other issues mentioned.

I don't think it's as easy as you make it sound. With inotify you
still need a syscall to determine whether there's an event on the
inotify fd and read it. Non-blocking read() might be faster than
stat() but it's still at least 500 times slower then just checking
that the environment variable hasn't changed. There may be a way to
wire up a signal to fire when the inotify fd has data ready, but then
you consume an extra signal for implementation-internal use and have
to deal with managing it.

> >In order for asynchronous timezone changes to be safe, the API would
> >have to be such that you make one call to get the current timezone,
> >then pass it as an argument to functions depending on the timezone, so
> >that the application has control of which timezone is getting used in
> >each call.
> 
> This is how the WIN32 API is implemented.

The glibc developers have discussed a new time[zone] API where
timezones are data objects rather than hidden global state. It might
be worth reading up on what they have in mind and if there are
improvements that should be suggested, and if the results could be
proposed as a standard.

Rich

Re: detect timezone changes by monitoring /etc/localtime (like glibc)

[Patrick Oppenlander]

On 13/06/17 09:19, Rich Felker wrote:
> On Tue, Jun 13, 2017 at 09:08:15AM +1000, Patrick Oppenlander wrote:
> 
>> Performance could be addressed (inotify?) but it's all moot anyway given
>> the other issues mentioned.
> 
> I don't think it's as easy as you make it sound. With inotify you
> still need a syscall to determine whether there's an event on the
> inotify fd and read it. Non-blocking read() might be faster than
> stat() but it's still at least 500 times slower then just checking
> that the environment variable hasn't changed. There may be a way to
> wire up a signal to fire when the inotify fd has data ready, but then
> you consume an extra signal for implementation-internal use and have
> to deal with managing it.
> 

I'm fairly sure that signal notifications on inotify handles were
implemented in 2.6 at some stage, however, I didn't consider the further
implications of doing such a thing.

> 
> The glibc developers have discussed a new time[zone] API where
> timezones are data objects rather than hidden global state. It might
> be worth reading up on what they have in mind and if there are
> improvements that should be suggested, and if the results could be
> proposed as a standard.
> 

I'm not signed up to the glibc lists, but I'll see what I can dig up.

On another note, while reading the code in this area I noticed that
musl mmap's the timezone file and holds pointers to the mapped area.
Is this safe given that system updates often change these files? Or
do we assume that no package manager is dumb enough to modify files
in place?

		Patrick

Re: detect timezone changes by monitoring /etc/localtime (like glibc)

[Rich Felker]

On Tue, Jun 13, 2017 at 09:49:59AM +1000, Patrick Oppenlander wrote:
> On another note, while reading the code in this area I noticed that
> musl mmap's the timezone file and holds pointers to the mapped area.
> Is this safe given that system updates often change these files? Or
> do we assume that no package manager is dumb enough to modify files
> in place?

I'm aware of that. Ideally we'd be using MAP_COPY, but Linux lacks it.
Note that even if we read the files into memory, though, there'd be a
race aginst modifying them at the same time. The only safe way to
install replacements for shared files is with atomic replacement
(rename). It's actually much more dangerous to modify executables or
shared libraries in place, so hopefully package managers are already
getting this right and applying the same solution to both executable
and non-executable files.

Rich