always reset DST rules during tzset

Submitted by Benjamin Peterson on Sept. 14, 2018, 10:46 p.m.

Details

Message ID 20180914224655.24718-1-benjamin@python.org
State New
Series "always reset DST rules during tzset"
Headers show

Commit Message

Benjamin Peterson Sept. 14, 2018, 10:46 p.m.
do_tzset() did't always reset the DST transition rules r0 and r1. That means the
rules from older TZ settings could leak into newer ones.

The following program demonstrates this bug. It should print out the same
timezone twice but doesn't due to the leaky state.

int main() {
	time_t t = 0;
	struct tm p;
	setenv("TZ", "STD-1DST", 1);
	localtime_r(&t, &p);
	printf("%s\n", p.tm_zone);
	setenv("TZ", "STD-1DST,M3.2.0,M11.1.0", 1);
	tzset();
	setenv("TZ", "STD-1DST", 1);
	localtime_r(&t, &p);
	printf("%s\n", p.tm_zone);
	return 0;
}
---
 src/time/__tz.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Patch hide | download patch | download mbox

diff --git a/src/time/__tz.c b/src/time/__tz.c
index 51e66514..3ccfdd5d 100644
--- a/src/time/__tz.c
+++ b/src/time/__tz.c
@@ -130,6 +130,9 @@  static void do_tzset()
 
 	if (old_tz && !strcmp(s, old_tz)) return;
 
+	memset(r0, 0, sizeof r0);
+	memset(r1, 0, sizeof r1);
+
 	if (zi) __munmap((void *)zi, map_size);
 
 	/* Cache the old value of TZ to check if it has changed. Avoid
@@ -194,7 +197,6 @@  static void do_tzset()
 			const unsigned char *p;
 			__tzname[0] = __tzname[1] = 0;
 			__daylight = __timezone = dst_off = 0;
-			for (i=0; i<5; i++) r0[i] = r1[i] = 0;
 			for (p=types; p<abbrevs; p+=6) {
 				if (!p[4] && !__tzname[0]) {
 					__tzname[0] = (char *)abbrevs + p[5];

Comments

Rich Felker Sept. 15, 2018, 1:16 a.m.
On Fri, Sep 14, 2018 at 03:46:55PM -0700, Benjamin Peterson wrote:
> do_tzset() did't always reset the DST transition rules r0 and r1. That means the
> rules from older TZ settings could leak into newer ones.
> 
> The following program demonstrates this bug. It should print out the same
> timezone twice but doesn't due to the leaky state.
> 
> int main() {
> 	time_t t = 0;
> 	struct tm p;
> 	setenv("TZ", "STD-1DST", 1);
> 	localtime_r(&t, &p);
> 	printf("%s\n", p.tm_zone);
> 	setenv("TZ", "STD-1DST,M3.2.0,M11.1.0", 1);
> 	tzset();
> 	setenv("TZ", "STD-1DST", 1);
> 	localtime_r(&t, &p);
> 	printf("%s\n", p.tm_zone);
> 	return 0;
> }
> ---
>  src/time/__tz.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/src/time/__tz.c b/src/time/__tz.c
> index 51e66514..3ccfdd5d 100644
> --- a/src/time/__tz.c
> +++ b/src/time/__tz.c
> @@ -130,6 +130,9 @@ static void do_tzset()
>  
>  	if (old_tz && !strcmp(s, old_tz)) return;
>  
> +	memset(r0, 0, sizeof r0);
> +	memset(r1, 0, sizeof r1);
> +
>  	if (zi) __munmap((void *)zi, map_size);
>  
>  	/* Cache the old value of TZ to check if it has changed. Avoid
> @@ -194,7 +197,6 @@ static void do_tzset()
>  			const unsigned char *p;
>  			__tzname[0] = __tzname[1] = 0;
>  			__daylight = __timezone = dst_off = 0;
> -			for (i=0; i<5; i++) r0[i] = r1[i] = 0;
>  			for (p=types; p<abbrevs; p+=6) {
>  				if (!p[4] && !__tzname[0]) {
>  					__tzname[0] = (char *)abbrevs + p[5];
> -- 
> 2.17.1

This looks right, but is there a reason you swapped the for loop out
for memsets? Either should work, but I probably preferred the loop in
the original since, due to -ffreestanding which is necessary when
building a libc, the compiler can't inline the memset automatically
with a builtin. I might however use the new header wrapper framework
to make it so (dependent on __GNUC__) memcpy and memset get redirected
to the __builtin_* versions except in the source files that define
them, in which case this kind of thing wouldn't matter.

Rich
Benjamin Peterson Sept. 15, 2018, 5:20 p.m.
> This looks right, but is there a reason you swapped the for loop out
> for memsets?

I thought it might generate better code due to store merging. But I definitely
forgot about -ffreestanding.

My latest patch switches back to the loop.