Re: Segfault in libthr.so on 9.0-BETA2 (with stunnel FWIW)

From: Kostik Belousov <kostikbel_at_gmail.com>
Date: Wed, 14 Sep 2011 23:04:56 +0300
On Wed, Sep 14, 2011 at 05:42:21PM +0200, Jeremie Le Hen wrote:
> On Wed, Sep 14, 2011 at 03:59:53PM +0300, Kostik Belousov wrote:
> > On Wed, Sep 14, 2011 at 02:36:07PM +0200, Jeremie Le Hen wrote:
> > > Hi list,
> > > 
> > > I've recently migrated my services from a box running 8.1-STABLE to
> > > another one running 9.0-BETA2.
> > > 
> > > I run stunnel 4.28 on 8.1-STABLE, and it has run flawlessly so far.  I
> > > compiled manually this very version on 9.0-BETA2.  But I get the
> > > following segfault:
> > > 
> > >     Program received signal SIGSEGV, Segmentation fault.
> > >     [Switching to Thread 803008c00 (LWP 100496/stunnel)]
> > >     0x000000080110d359 in gmtime_r () from /lib/libc.so.7
> > >     (gdb) thread
> > >     [Current thread is 3 (Thread 803008c00 (LWP 100496/stunnel))]
> > >     (gdb) bt
> > >     #0  0x000000080110d359 in gmtime_r () from /lib/libc.so.7
> > >     #1  0x000000080110cdde in gmtime_r () from /lib/libc.so.7
> > >     #2  0x000000080110dab4 in gmtime_r () from /lib/libc.so.7
> > >     #3  0x000000080110dcc8 in gmtime_r () from /lib/libc.so.7
> > >     #4  0x0000000800e1d9e8 in pthread_once () from /lib/libthr.so.3
> > >     #5  0x000000080110ca9f in timegm () from /lib/libc.so.7
> > >     #6  0x0000000805dff8d9 in OPENSSL_gmtime () from /usr/local/lib/libcrypto.so.7
> > >     #7  0x0000000805e74631 in ASN1_UTCTIME_adj () from /usr/local/lib/libcrypto.so.7
> > >     #8  0x0000000805e9462d in X509_time_adj_ex () from /usr/local/lib/libcrypto.so.7
> > >     #9  0x0000000805e9478c in X509_cmp_time () from /usr/local/lib/libcrypto.so.7
> > >     #10 0x0000000805e9496d in internal_verify () from /usr/local/lib/libcrypto.so.7
> > >     #11 0x0000000805e95f46 in X509_verify_cert () from /usr/local/lib/libcrypto.so.7
> > >     #12 0x0000000805b7f4c8 in ssl_verify_cert_chain () from /usr/local/lib/libssl.so.7
> > >     #13 0x0000000805b5d6e3 in ssl3_get_client_certificate () from /usr/local/lib/libssl.so.7
> > >     #14 0x0000000805b612bc in ssl3_accept () from /usr/local/lib/libssl.so.7
> > >     #15 0x0000000000406f6e in init_ssl (c=0x803093000) at client.c:329
> > >     #16 0x00000000004069a6 in do_client (c=0x803093000) at client.c:202
> > >     #17 0x000000000040676b in run_client (c=0x803093000) at client.c:150
> > >     #18 0x00000000004066cf in client (arg=0x803093000) at client.c:123
> > >     #19 0x0000000800e18224 in pthread_getprio () from /lib/libthr.so.3
> > >     #20 0x0000000000000000 in ?? ()
> > > 
> > > 
> > > Note that I tried with the newest version of stunnel, it crashes at the
> > > same place.  I also tried libssl.so both from the base system and from
> > > the ports, same thing.
> > 
> > You need to compile both libc and libthr with debugging symbols and
> > do a backtrace with such libraries.
> 
> Here it is:
> 
> #0  0x0000000807509359 in tzload (name=0x807536281 "posixrules", sp=0x7fffffbf8e80, doextend=0)
>     at /usr/src/lib/libc/../../contrib/tzcode/stdtime/localtime.c:393
> #1  0x0000000807508dde in tzparse (name=0x7fffffbeec95 "", sp=0x7fffffbf8e80, lastditch=Variable "lastditch" is not available.
> )
>     at /usr/src/lib/libc/../../contrib/tzcode/stdtime/localtime.c:1001
> #2  0x0000000807509ab4 in tzload (name=Variable "name" is not available.
> ) at /usr/src/lib/libc/../../contrib/tzcode/stdtime/localtime.c:579
> #3  0x0000000807509cc8 in gmtload (sp=0x80776f6c0) at /usr/src/lib/libc/../../contrib/tzcode/stdtime/localtime.c:1196
> #4  0x000000080ce5d9e8 in _pthread_once (once_control=0x80776af00, init_routine=0x807509cf0 <gmt_init>)
>     at /usr/src/lib/libthr/thread/thr_once.c:87
> #5  0x0000000807508a9f in gmtsub (timep=0x7fffffbfdaf8, offset=0, tmp=0x7fffffbfdb00)
>     at /usr/src/lib/libc/../../contrib/tzcode/stdtime/localtime.c:1485
> #6  0x000000080e7e58d9 in OPENSSL_gmtime () from /usr/local/lib/libcrypto.so.7
> #7  0x000000080e85a631 in ASN1_UTCTIME_adj () from /usr/local/lib/libcrypto.so.7
> #8  0x000000080e87a62d in X509_time_adj_ex () from /usr/local/lib/libcrypto.so.7
> #9  0x000000080e87a78c in X509_cmp_time () from /usr/local/lib/libcrypto.so.7
> #10 0x000000080e87a96d in internal_verify () from /usr/local/lib/libcrypto.so.7
> #11 0x000000080e87bf46 in X509_verify_cert () from /usr/local/lib/libcrypto.so.7
> #12 0x0000000809ec94c8 in ssl_verify_cert_chain () from /usr/local/lib/libssl.so.7
> #13 0x0000000809ea76e3 in ssl3_get_client_certificate () from /usr/local/lib/libssl.so.7
> #14 0x0000000809eab2bc in ssl3_accept () from /usr/local/lib/libssl.so.7
> #15 0x00000000004076f9 in ?? ()
> #16 0x00000000004082cf in ?? ()
> #17 0x0000000000408c50 in ?? ()
> #18 0x0000000000408d17 in ?? ()
> #19 0x000000080ce58224 in thread_start (curthread=0x80d408c00) at /usr/src/lib/libthr/thread/thr_create.c:284
> #20 0x0000000000000000 in ?? ()
> Error accessing memory address 0x7fffffbfe000: Bad address.

tzload() allocates ~80KB for the local variables. The backtrace you provided
shows the nested call to tzload(), so there is total 160KB of the stack
space consumed.

By default, stack for the amd64 thread is 4MB, that should be plenty. This
is not the case for ezm3. Possibly, stunnel also reduces the size of the
thread stack.

Please, try the patch below. I did not tested it, only compiled. I see
that now tzload allocates only ~300 bytes on the stack.

diff --git a/contrib/tzcode/stdtime/localtime.c b/contrib/tzcode/stdtime/localtime.c
index 80b70ac..55d55e0 100644
--- a/contrib/tzcode/stdtime/localtime.c
+++ b/contrib/tzcode/stdtime/localtime.c
_at__at_ -380,13 +380,16 _at__at_ register const int	doextend;
 	int		fid;
 	int		stored;
 	int		nread;
+	int		res;
 	union {
 		struct tzhead	tzhead;
 		char		buf[2 * sizeof(struct tzhead) +
 					2 * sizeof *sp +
 					4 * TZ_MAX_TIMES];
-	} u;
+	} *u;
 
+	u = NULL;
+	res = -1;
 	sp->goback = sp->goahead = FALSE;
 
 	/* XXX The following is from OpenBSD, and I'm not sure it is correct */
_at__at_ -406,16 +409,24 _at__at_ register const int	doextend;
 		** to hold the longest file name string that the implementation
 		** guarantees can be opened."
 		*/
-		char		fullname[FILENAME_MAX + 1];
+		char		*fullname;
+
+		fullname = malloc(FILENAME_MAX + 1);
+		if (fullname == NULL)
+			goto out;
 
 		if (name[0] == ':')
 			++name;
 		doaccess = name[0] == '/';
 		if (!doaccess) {
-			if ((p = TZDIR) == NULL)
+			if ((p = TZDIR) == NULL) {
+				free(fullname);
 				return -1;
-			if ((strlen(p) + 1 + strlen(name) + 1) >= sizeof fullname)
+			}
+			if ((strlen(p) + 1 + strlen(name) + 1) >= sizeof fullname) {
+				free(fullname);
 				return -1;
+			}
 			(void) strcpy(fullname, p);
 			(void) strcat(fullname, "/");
 			(void) strcat(fullname, name);
_at__at_ -426,37 +437,45 _at__at_ register const int	doextend;
 				doaccess = TRUE;
 			name = fullname;
 		}
-		if (doaccess && access(name, R_OK) != 0)
+		if (doaccess && access(name, R_OK) != 0) {
+			free(fullname);
 		     	return -1;
-		if ((fid = _open(name, OPEN_MODE)) == -1)
+		}
+		if ((fid = _open(name, OPEN_MODE)) == -1) {
+			free(fullname);
 			return -1;
+		}
 		if ((_fstat(fid, &stab) < 0) || !S_ISREG(stab.st_mode)) {
+			free(fullname);
 			_close(fid);
 			return -1;
 		}
 	}
-	nread = _read(fid, u.buf, sizeof u.buf);
+	u = malloc(sizeof(*u));
+	if (u == NULL)
+		goto out;
+	nread = _read(fid, u->buf, sizeof u->buf);
 	if (_close(fid) < 0 || nread <= 0)
-		return -1;
+		goto out;
 	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 out;
+		if (nread - (p - u->buf) <
 			sp->timecnt * stored +		/* ats */
 			sp->timecnt +			/* types */
 			sp->typecnt * 6 +		/* ttinfos */
_at__at_ -464,7 +483,7 _at__at_ register const int	doextend;
 			sp->leapcnt * (stored + 4) +	/* lsinfos */
 			ttisstdcnt +			/* ttisstds */
 			ttisgmtcnt)			/* ttisgmts */
-				return -1;
+				goto out;
 		for (i = 0; i < sp->timecnt; ++i) {
 			sp->ats[i] = (stored == 4) ?
 				detzcode(p) : detzcode64(p);
_at__at_ -473,7 +492,7 _at__at_ register const int	doextend;
 		for (i = 0; i < sp->timecnt; ++i) {
 			sp->types[i] = (unsigned char) *p++;
 			if (sp->types[i] >= sp->typecnt)
-				return -1;
+				goto out;
 		}
 		for (i = 0; i < sp->typecnt; ++i) {
 			struct ttinfo *	ttisp;
_at__at_ -483,11 +502,11 _at__at_ register const int	doextend;
 			p += 4;
 			ttisp->tt_isdst = (unsigned char) *p++;
 			if (ttisp->tt_isdst != 0 && ttisp->tt_isdst != 1)
-				return -1;
+				goto out;
 			ttisp->tt_abbrind = (unsigned char) *p++;
 			if (ttisp->tt_abbrind < 0 ||
 				ttisp->tt_abbrind > sp->charcnt)
-					return -1;
+					goto out;
 		}
 		for (i = 0; i < sp->charcnt; ++i)
 			sp->chars[i] = *p++;
_at__at_ -512,7 +531,7 _at__at_ register const int	doextend;
 				ttisp->tt_ttisstd = *p++;
 				if (ttisp->tt_ttisstd != TRUE &&
 					ttisp->tt_ttisstd != FALSE)
-						return -1;
+						goto out;
 			}
 		}
 		for (i = 0; i < sp->typecnt; ++i) {
_at__at_ -525,7 +544,7 _at__at_ register const int	doextend;
 				ttisp->tt_ttisgmt = *p++;
 				if (ttisp->tt_ttisgmt != TRUE &&
 					ttisp->tt_ttisgmt != FALSE)
-						return -1;
+						goto out;
 			}
 		}
 		/*
_at__at_ -558,11 +577,11 _at__at_ register const int	doextend;
 		/*
 		** 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.
 		*/
_at__at_ -570,39 +589,43 _at__at_ register const int	doextend;
 			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;
+			struct state	*ts;
 			register int	result;
 
-			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) {
+			ts = malloc(sizeof(*ts));
+			if (ts == NULL)
+				goto out;
+			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)
-						ts.ttis[i].tt_abbrind +=
+						ts->ttis[i].tt_abbrind +=
 							sp->charcnt;
-					for (i = 0; i < ts.charcnt; ++i)
+					for (i = 0; i < ts->charcnt; ++i)
 						sp->chars[sp->charcnt++] =
-							ts.chars[i];
+							ts->chars[i];
 					i = 0;
-					while (i < ts.timecnt &&
-						ts.ats[i] <=
+					while (i < ts->timecnt &&
+						ts->ats[i] <=
 						sp->ats[sp->timecnt - 1])
 							++i;
-					while (i < ts.timecnt &&
+					while (i < ts->timecnt &&
 					    sp->timecnt < TZ_MAX_TIMES) {
 						sp->ats[sp->timecnt] =
-							ts.ats[i];
+							ts->ats[i];
 						sp->types[sp->timecnt] =
 							sp->typecnt +
-							ts.types[i];
+							ts->types[i];
 						++sp->timecnt;
 						++i;
 					}
-					sp->ttis[sp->typecnt++] = ts.ttis[0];
-					sp->ttis[sp->typecnt++] = ts.ttis[1];
+					sp->ttis[sp->typecnt++] = ts->ttis[0];
+					sp->ttis[sp->typecnt++] = ts->ttis[1];
 			}
+			free(ts);
 	}
 	if (sp->timecnt > 1) {
 		for (i = 1; i < sp->timecnt; ++i)
_at__at_ -620,7 +643,10 _at__at_ register const int	doextend;
 					break;
 		}
 	}
-	return 0;
+	res = 0;
+out:
+	free(u);
+	return (res);
 }
 
 static int

Received on Wed Sep 14 2011 - 18:05:11 UTC

This archive was generated by hypermail 2.4.0 : Wed May 19 2021 - 11:40:17 UTC