Re: [musl] Handling of non-location specific TZ values

[Rich Felker <dalias@libc.org>]

On Wed, Jun 02, 2021 at 05:41:30PM +0200, Sören Tempel wrote:
> Ping.
> 
> Would be willing to adjust the patch as needed. In any case, it would be
> nice to get this fixed as it currently causes some test failures of
> Alpine packages.

Thanks for the ping. I had an alternate approach in draft that I want
to look back at and compare. I'll try to get this fixed one way or the
other before rolling the release.

Rich


> Sören Tempel <soeren@soeren-tempel.net> wrote:
> > Rich Felker <dalias@libc.org> wrote:
> > > Yes. I suspect we can get by with calling getname with a dummy output
> > > array, then checking if the next character is one of +, -, or a digit.
> > > If not (in particular, if it's a null character) then we can attempt
> > > loading it as a file.
> > 
> > Maybe something along the following? Not too familiar with the musl code
> > base so not sure if including ctype.h is allowed etc.
> > 
> > diff --git a/src/time/__tz.c b/src/time/__tz.c
> > index 09a6317e..6bc183d0 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 <ctype.h>
> >  #include <sys/mman.h>
> >  #include "libc.h"
> >  #include "lock.h"
> > @@ -125,13 +126,13 @@ static size_t zi_dotprod(const unsigned char *z, const unsigned char *v, size_t
> >  static void do_tzset()
> >  {
> >  	char buf[NAME_MAX+25], *pathname=buf+24;
> > -	const char *try, *s, *p;
> > +	const char *try, *s, *orig;
> >  	const unsigned char *map = 0;
> >  	size_t i;
> >  	static const char search[] =
> >  		"/usr/share/zoneinfo/\0/share/zoneinfo/\0/etc/zoneinfo/\0";
> > 
> > -	s = getenv("TZ");
> > +	s = orig = getenv("TZ");
> >  	if (!s) s = "/etc/localtime";
> >  	if (!*s) s = __utc;
> > 
> > @@ -154,11 +155,19 @@ static void do_tzset()
> >  	}
> >  	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
> > -	 * standard path will be searched. */
> > -	if (*s == ':' || ((p=strchr(s, '/')) && !memchr(s, ',', p-s))) {
> > +	/* The TZ format specified by POSIX consists of a mandatory
> > +	 * time zone name and a mandatory offset. We determine the
> > +	 * name using getname, if the next character cannot constitute
> > +	 * a valid offset (or the TZ value starts with a colon) we
> > +	 * interpret the TZ environment variable as a zoneinfo file name. */
> > +	getname(std_name, &s);
> > +	if (*s == ':' || (!isdigit(*s) && *s != '+' && *s != '-')) {
> >  		if (*s == ':') s++;
> > +		else if (orig) s = orig;
> > +
> > +		/* Non-suid can use an absolute tzfile pathname or a relative
> > +		 * pathame beginning with "."; in secure mode, only the
> > +		 * standard path will be searched. */
> >  		if (*s == '/' || *s == '.') {
> >  			if (!libc.secure || !strcmp(s, "/etc/localtime"))
> >  				map = __map_file(s, &map_size);
> > 
> > Patch can be tested using date(1). For instance, compare the output of
> > `TZ=CET date` on a patched and unpatched system with an installed
> > zoneinfo database.
> > 
> > > It might be worth adding a special exception for "UTC" and "GMT" so
> > > that they are always interpreted as "UTC0" and "GMT0" and can't be
> > > overridden by a bogus file in the zoneinfo path, for the sake of
> > > software that does "TZ=UTC cmd" to avoid any timezone shenanigans.
> > 
> > Maybe that can be done in a separate commit?
> > 
> > Greetings,
> > Sören