FW: stack overflow in tzload

[walter harms]

what si about using alloc() ?
System that do not provied it can use malloc,
others will use the stack but we will take the stack only when
needed and get an error (hopefully) when oom.

re,
 wh


Am 23.11.2010 00:56, schrieb Olson, Arthur David (NIH/NCI) [E]:
> I'm forwarding this message from Ted Unangst, who is not on the time zone mailing list. Those of you who are on the list, please direct replies appropriately.
> 
> 				--ado
> 
> -----Original Message-----
> From: Ted Unangst [mailto:ted.unangst at gmail.com] 
> Sent: Monday, November 22, 2010 6:22
> To: tz at lecserver.nci.nih.gov
> Subject: stack overflow in tzload
> 
> Stack overflow as in there's not enough stack.  I ran into this problem 
> running a program on openbsd that was using a fairly small thread size.  I 
> switched to using malloc.
> 
> Index: localtime.c
> ===================================================================
> RCS file: /home/tedu/cvs/src/lib/libc/time/localtime.c,v
> retrieving revision 1.35
> diff -u -r1.35 localtime.c
> --- localtime.c	23 Aug 2010 22:35:34 -0000	1.35
> +++ localtime.c	20 Nov 2010 23:32:09 -0000
> @@ -340,7 +340,7 @@
>  		char		buf[2 * sizeof(struct tzhead) +
>  					2 * sizeof *sp +
>  					4 * TZ_MAX_TIMES];
> -	} u;
> +	} *u;
>  
>  	sp->goback = sp->goahead = FALSE;
>  	if (name != NULL && issetugid() != 0)
> @@ -383,28 +383,33 @@
>  		if ((fid = open(name, OPEN_MODE)) == -1)
>  			return -1;
>  	}
> -	nread = read(fid, u.buf, sizeof u.buf);
> -	if (close(fid) < 0 || nread <= 0)
> +	u = malloc(sizeof(*u));
> +	if (!u) {
> +		close(fid);
>  		return -1;
> +	}
> +	nread = read(fid, u->buf, sizeof u->buf);
> +	if (close(fid) < 0 || nread <= 0)
> +		goto bad;
>  	for (stored = 4; stored <= 8; stored *= 2) {
>  		int		ttisstdcnt;
>  		int		ttisgmtcnt;
>  
> -		ttisstdcnt = (int) detzcode(u.tzhead.tzh_ttisstdcnt);
> -		ttisgmtcnt = (int) detzcode(u.tzhead.tzh_ttisgmtcnt);
> -		sp->leapcnt = (int) detzcode(u.tzhead.tzh_leapcnt);
> -		sp->timecnt = (int) detzcode(u.tzhead.tzh_timecnt);
> -		sp->typecnt = (int) detzcode(u.tzhead.tzh_typecnt);
> -		sp->charcnt = (int) detzcode(u.tzhead.tzh_charcnt);
> -		p = u.tzhead.tzh_charcnt + sizeof u.tzhead.tzh_charcnt;
> +		ttisstdcnt = (int) detzcode(u->tzhead.tzh_ttisstdcnt);
> +		ttisgmtcnt = (int) detzcode(u->tzhead.tzh_ttisgmtcnt);
> +		sp->leapcnt = (int) detzcode(u->tzhead.tzh_leapcnt);
> +		sp->timecnt = (int) detzcode(u->tzhead.tzh_timecnt);
> +		sp->typecnt = (int) detzcode(u->tzhead.tzh_typecnt);
> +		sp->charcnt = (int) detzcode(u->tzhead.tzh_charcnt);
> +		p = u->tzhead.tzh_charcnt + sizeof u->tzhead.tzh_charcnt;
>  		if (sp->leapcnt < 0 || sp->leapcnt > TZ_MAX_LEAPS ||
>  			sp->typecnt <= 0 || sp->typecnt > TZ_MAX_TYPES ||
>  			sp->timecnt < 0 || sp->timecnt > TZ_MAX_TIMES ||
>  			sp->charcnt < 0 || sp->charcnt > TZ_MAX_CHARS ||
>  			(ttisstdcnt != sp->typecnt && ttisstdcnt != 0) ||
>  			(ttisgmtcnt != sp->typecnt && ttisgmtcnt != 0))
> -				return -1;
> -		if (nread - (p - u.buf) <
> +				goto bad;
> +		if (nread - (p - u->buf) <
>  			sp->timecnt * stored +		/* ats */
>  			sp->timecnt +			/* types */
>  			sp->typecnt * 6 +		/* ttinfos */
> @@ -412,7 +417,7 @@
>  			sp->leapcnt * (stored + 4) +	/* lsinfos */
>  			ttisstdcnt +			/* ttisstds */
>  			ttisgmtcnt)			/* ttisgmts */
> -				return -1;
> +				goto bad;
>  		for (i = 0; i < sp->timecnt; ++i) {
>  			sp->ats[i] = (stored == 4) ?
>  				detzcode(p) : detzcode64(p);
> @@ -421,7 +426,7 @@
>  		for (i = 0; i < sp->timecnt; ++i) {
>  			sp->types[i] = (unsigned char) *p++;
>  			if (sp->types[i] >= sp->typecnt)
> -				return -1;
> +				goto bad;
>  		}
>  		for (i = 0; i < sp->typecnt; ++i) {
>  			register struct ttinfo *	ttisp;
> @@ -431,11 +436,11 @@
>  			p += 4;
>  			ttisp->tt_isdst = (unsigned char) *p++;
>  			if (ttisp->tt_isdst != 0 && ttisp->tt_isdst != 1)
> -				return -1;
> +				goto bad;
>  			ttisp->tt_abbrind = (unsigned char) *p++;
>  			if (ttisp->tt_abbrind < 0 ||
>  				ttisp->tt_abbrind > sp->charcnt)
> -					return -1;
> +					goto bad;
>  		}
>  		for (i = 0; i < sp->charcnt; ++i)
>  			sp->chars[i] = *p++;
> @@ -460,7 +465,7 @@
>  				ttisp->tt_ttisstd = *p++;
>  				if (ttisp->tt_ttisstd != TRUE &&
>  					ttisp->tt_ttisstd != FALSE)
> -						return -1;
> +						goto bad;
>  			}
>  		}
>  		for (i = 0; i < sp->typecnt; ++i) {
> @@ -473,7 +478,7 @@
>  				ttisp->tt_ttisgmt = *p++;
>  				if (ttisp->tt_ttisgmt != TRUE &&
>  					ttisp->tt_ttisgmt != FALSE)
> -						return -1;
> +						goto bad;
>  			}
>  		}
>  		/*
> @@ -506,11 +511,11 @@
>  		/*
>  		** If this is an old file, we're done.
>  		*/
> -		if (u.tzhead.tzh_version[0] == '\0')
> +		if (u->tzhead.tzh_version[0] == '\0')
>  			break;
> -		nread -= p - u.buf;
> +		nread -= p - u->buf;
>  		for (i = 0; i < nread; ++i)
> -			u.buf[i] = p[i];
> +			u->buf[i] = p[i];
>  		/*
>  		** If this is a narrow integer time_t system, we're done.
>  		*/
> @@ -518,13 +523,13 @@
>  			break;
>  	}
>  	if (doextend && nread > 2 &&
> -		u.buf[0] == '\n' && u.buf[nread - 1] == '\n' &&
> +		u->buf[0] == '\n' && u->buf[nread - 1] == '\n' &&
>  		sp->typecnt + 2 <= TZ_MAX_TYPES) {
>  			struct state	ts;
>  			register int	result;
>  
> -			u.buf[nread - 1] = '\0';
> -			result = tzparse(&u.buf[1], &ts, FALSE);
> +			u->buf[nread - 1] = '\0';
> +			result = tzparse(&u->buf[1], &ts, FALSE);
>  			if (result == 0 && ts.typecnt == 2 &&
>  				sp->charcnt + ts.charcnt <= TZ_MAX_CHARS) {
>  					for (i = 0; i < 2; ++i)
> @@ -568,7 +573,11 @@
>  					break;
>  		}
>  	}
> +	free(u);
>  	return 0;
> +bad:
> +	free(u);
> +	return -1;
>  }
>  
>  static int
> 
> 
> 
>