Date: Sun, 18 Sep 2011 18:56:49 +0300 From: Kostik Belousov <kostikbel@gmail.com> To: Jilles Tjoelker <jilles@stack.nl> Cc: Peter Pentchev <roam@ringlet.net>, freebsd-current@freebsd.org, Jeremie Le Hen <jeremie@le-hen.org>, David Xu <davidxu@freebsd.org>, Oliver Lehmann <lehmann@ans-netz.de> Subject: Re: Segfault in libthr.so on 9.0-BETA2 (with stunnel FWIW) Message-ID: <20110918155649.GY1511@deviant.kiev.zoral.com.ua> In-Reply-To: <20110918115650.GA36162@stack.nl> References: <20110914123607.GM65366@felucia.tataz.chchile.org> <20110914125953.GX17489@deviant.kiev.zoral.com.ua> <20110914154221.GB7863@felucia.tataz.chchile.org> <20110914200456.GE17489@deviant.kiev.zoral.com.ua> <20110918115650.GA36162@stack.nl>
next in thread | previous in thread | raw e-mail | index | archive | help
--6749iDyxihQ87e9v
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
On Sun, Sep 18, 2011 at 01:56:50PM +0200, Jilles Tjoelker wrote:
> On Wed, Sep 14, 2011 at 11:04:56PM +0300, Kostik Belousov wrote:
> > tzload() allocates ~80KB for the local variables. The backtrace you pro=
vided
> > shows the nested call to tzload(), so there is total 160KB of the stack
> > space consumed.
>=20
> > By default, stack for the amd64 thread is 4MB, that should be plenty. T=
his
> > is not the case for ezm3. Possibly, stunnel also reduces the size of the
> > thread stack.
>=20
> > Please, try the patch below. I did not tested it, only compiled. I see
> > that now tzload allocates only ~300 bytes on the stack.
>=20
> 80KB seems quite a lot indeed, good to bring it down.
>=20
> > diff --git a/contrib/tzcode/stdtime/localtime.c b/contrib/tzcode/stdtim=
e/localtime.c
> > index 80b70ac..55d55e0 100644
> > --- a/contrib/tzcode/stdtime/localtime.c
> > +++ b/contrib/tzcode/stdtime/localtime.c
> [snip]
> > @@ -406,16 +409,24 @@ 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 =3D malloc(FILENAME_MAX + 1);
> > + if (fullname =3D=3D NULL)
> > + goto out;
> > =20
> > if (name[0] =3D=3D ':')
> > ++name;
> > doaccess =3D name[0] =3D=3D '/';
> > if (!doaccess) {
> > - if ((p =3D TZDIR) =3D=3D NULL)
> > + if ((p =3D TZDIR) =3D=3D NULL) {
> > + free(fullname);
> > return -1;
> > - if ((strlen(p) + 1 + strlen(name) + 1) >=3D sizeof fullname)
> > + }
> > + if ((strlen(p) + 1 + strlen(name) + 1) >=3D sizeof fullname) {
>=20
> This sizeof is now the sizeof of a pointer. The comparison should be
> against FILENAME_MAX + 1 instead.
>=20
> Alternatively, the name could be created using asprintf().
You are right. I fixed the defect.
Updated patch below.
diff --git a/contrib/tzcode/stdtime/localtime.c b/contrib/tzcode/stdtime/lo=
caltime.c
index 80b70ac..b1981b6 100644
--- a/contrib/tzcode/stdtime/localtime.c
+++ b/contrib/tzcode/stdtime/localtime.c
@@ -380,13 +380,16 @@ 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;
=20
+ u =3D NULL;
+ res =3D -1;
sp->goback =3D sp->goahead =3D FALSE;
=20
/* XXX The following is from OpenBSD, and I'm not sure it is correct */
@@ -406,16 +409,24 @@ 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 =3D malloc(FILENAME_MAX + 1);
+ if (fullname =3D=3D NULL)
+ goto out;
=20
if (name[0] =3D=3D ':')
++name;
doaccess =3D name[0] =3D=3D '/';
if (!doaccess) {
- if ((p =3D TZDIR) =3D=3D NULL)
+ if ((p =3D TZDIR) =3D=3D NULL) {
+ free(fullname);
return -1;
- if ((strlen(p) + 1 + strlen(name) + 1) >=3D sizeof fullname)
+ }
+ if (strlen(p) + 1 + strlen(name) >=3D FILENAME_MAX) {
+ free(fullname);
return -1;
+ }
(void) strcpy(fullname, p);
(void) strcat(fullname, "/");
(void) strcat(fullname, name);
@@ -426,37 +437,45 @@ register const int doextend;
doaccess =3D TRUE;
name =3D fullname;
}
- if (doaccess && access(name, R_OK) !=3D 0)
+ if (doaccess && access(name, R_OK) !=3D 0) {
+ free(fullname);
return -1;
- if ((fid =3D _open(name, OPEN_MODE)) =3D=3D -1)
+ }
+ if ((fid =3D _open(name, OPEN_MODE)) =3D=3D -1) {
+ free(fullname);
return -1;
+ }
if ((_fstat(fid, &stab) < 0) || !S_ISREG(stab.st_mode)) {
+ free(fullname);
_close(fid);
return -1;
}
}
- nread =3D _read(fid, u.buf, sizeof u.buf);
+ u =3D malloc(sizeof(*u));
+ if (u =3D=3D NULL)
+ goto out;
+ nread =3D _read(fid, u->buf, sizeof u->buf);
if (_close(fid) < 0 || nread <=3D 0)
- return -1;
+ goto out;
for (stored =3D 4; stored <=3D 8; stored *=3D 2) {
int ttisstdcnt;
int ttisgmtcnt;
=20
- ttisstdcnt =3D (int) detzcode(u.tzhead.tzh_ttisstdcnt);
- ttisgmtcnt =3D (int) detzcode(u.tzhead.tzh_ttisgmtcnt);
- sp->leapcnt =3D (int) detzcode(u.tzhead.tzh_leapcnt);
- sp->timecnt =3D (int) detzcode(u.tzhead.tzh_timecnt);
- sp->typecnt =3D (int) detzcode(u.tzhead.tzh_typecnt);
- sp->charcnt =3D (int) detzcode(u.tzhead.tzh_charcnt);
- p =3D u.tzhead.tzh_charcnt + sizeof u.tzhead.tzh_charcnt;
+ ttisstdcnt =3D (int) detzcode(u->tzhead.tzh_ttisstdcnt);
+ ttisgmtcnt =3D (int) detzcode(u->tzhead.tzh_ttisgmtcnt);
+ sp->leapcnt =3D (int) detzcode(u->tzhead.tzh_leapcnt);
+ sp->timecnt =3D (int) detzcode(u->tzhead.tzh_timecnt);
+ sp->typecnt =3D (int) detzcode(u->tzhead.tzh_typecnt);
+ sp->charcnt =3D (int) detzcode(u->tzhead.tzh_charcnt);
+ p =3D u->tzhead.tzh_charcnt + sizeof u->tzhead.tzh_charcnt;
if (sp->leapcnt < 0 || sp->leapcnt > TZ_MAX_LEAPS ||
sp->typecnt <=3D 0 || sp->typecnt > TZ_MAX_TYPES ||
sp->timecnt < 0 || sp->timecnt > TZ_MAX_TIMES ||
sp->charcnt < 0 || sp->charcnt > TZ_MAX_CHARS ||
(ttisstdcnt !=3D sp->typecnt && ttisstdcnt !=3D 0) ||
(ttisgmtcnt !=3D sp->typecnt && ttisgmtcnt !=3D 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 */
@@ -464,7 +483,7 @@ register const int doextend;
sp->leapcnt * (stored + 4) + /* lsinfos */
ttisstdcnt + /* ttisstds */
ttisgmtcnt) /* ttisgmts */
- return -1;
+ goto out;
for (i =3D 0; i < sp->timecnt; ++i) {
sp->ats[i] =3D (stored =3D=3D 4) ?
detzcode(p) : detzcode64(p);
@@ -473,7 +492,7 @@ register const int doextend;
for (i =3D 0; i < sp->timecnt; ++i) {
sp->types[i] =3D (unsigned char) *p++;
if (sp->types[i] >=3D sp->typecnt)
- return -1;
+ goto out;
}
for (i =3D 0; i < sp->typecnt; ++i) {
struct ttinfo * ttisp;
@@ -483,11 +502,11 @@ register const int doextend;
p +=3D 4;
ttisp->tt_isdst =3D (unsigned char) *p++;
if (ttisp->tt_isdst !=3D 0 && ttisp->tt_isdst !=3D 1)
- return -1;
+ goto out;
ttisp->tt_abbrind =3D (unsigned char) *p++;
if (ttisp->tt_abbrind < 0 ||
ttisp->tt_abbrind > sp->charcnt)
- return -1;
+ goto out;
}
for (i =3D 0; i < sp->charcnt; ++i)
sp->chars[i] =3D *p++;
@@ -512,7 +531,7 @@ register const int doextend;
ttisp->tt_ttisstd =3D *p++;
if (ttisp->tt_ttisstd !=3D TRUE &&
ttisp->tt_ttisstd !=3D FALSE)
- return -1;
+ goto out;
}
}
for (i =3D 0; i < sp->typecnt; ++i) {
@@ -525,7 +544,7 @@ register const int doextend;
ttisp->tt_ttisgmt =3D *p++;
if (ttisp->tt_ttisgmt !=3D TRUE &&
ttisp->tt_ttisgmt !=3D FALSE)
- return -1;
+ goto out;
}
}
/*
@@ -558,11 +577,11 @@ register const int doextend;
/*
** If this is an old file, we're done.
*/
- if (u.tzhead.tzh_version[0] =3D=3D '\0')
+ if (u->tzhead.tzh_version[0] =3D=3D '\0')
break;
- nread -=3D p - u.buf;
+ nread -=3D p - u->buf;
for (i =3D 0; i < nread; ++i)
- u.buf[i] =3D p[i];
+ u->buf[i] =3D p[i];
/*
** If this is a narrow integer time_t system, we're done.
*/
@@ -570,39 +589,43 @@ register const int doextend;
break;
}
if (doextend && nread > 2 &&
- u.buf[0] =3D=3D '\n' && u.buf[nread - 1] =3D=3D '\n' &&
+ u->buf[0] =3D=3D '\n' && u->buf[nread - 1] =3D=3D '\n' &&
sp->typecnt + 2 <=3D TZ_MAX_TYPES) {
- struct state ts;
+ struct state *ts;
register int result;
=20
- u.buf[nread - 1] =3D '\0';
- result =3D tzparse(&u.buf[1], &ts, FALSE);
- if (result =3D=3D 0 && ts.typecnt =3D=3D 2 &&
- sp->charcnt + ts.charcnt <=3D TZ_MAX_CHARS) {
+ ts =3D malloc(sizeof(*ts));
+ if (ts =3D=3D NULL)
+ goto out;
+ u->buf[nread - 1] =3D '\0';
+ result =3D tzparse(&u->buf[1], ts, FALSE);
+ if (result =3D=3D 0 && ts->typecnt =3D=3D 2 &&
+ sp->charcnt + ts->charcnt <=3D TZ_MAX_CHARS) {
for (i =3D 0; i < 2; ++i)
- ts.ttis[i].tt_abbrind +=3D
+ ts->ttis[i].tt_abbrind +=3D
sp->charcnt;
- for (i =3D 0; i < ts.charcnt; ++i)
+ for (i =3D 0; i < ts->charcnt; ++i)
sp->chars[sp->charcnt++] =3D
- ts.chars[i];
+ ts->chars[i];
i =3D 0;
- while (i < ts.timecnt &&
- ts.ats[i] <=3D
+ while (i < ts->timecnt &&
+ ts->ats[i] <=3D
sp->ats[sp->timecnt - 1])
++i;
- while (i < ts.timecnt &&
+ while (i < ts->timecnt &&
sp->timecnt < TZ_MAX_TIMES) {
sp->ats[sp->timecnt] =3D
- ts.ats[i];
+ ts->ats[i];
sp->types[sp->timecnt] =3D
sp->typecnt +
- ts.types[i];
+ ts->types[i];
++sp->timecnt;
++i;
}
- sp->ttis[sp->typecnt++] =3D ts.ttis[0];
- sp->ttis[sp->typecnt++] =3D ts.ttis[1];
+ sp->ttis[sp->typecnt++] =3D ts->ttis[0];
+ sp->ttis[sp->typecnt++] =3D ts->ttis[1];
}
+ free(ts);
}
if (sp->timecnt > 1) {
for (i =3D 1; i < sp->timecnt; ++i)
@@ -620,7 +643,10 @@ register const int doextend;
break;
}
}
- return 0;
+ res =3D 0;
+out:
+ free(u);
+ return (res);
}
=20
static int
--6749iDyxihQ87e9v
Content-Type: application/pgp-signature
Content-Disposition: inline
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.18 (FreeBSD)
iEYEARECAAYFAk52FMEACgkQC3+MBN1Mb4gsYgCgvVypsbgE/rSIdiNeCk2D5oDx
1vsAn1Zpa4zeGTuXYWrB6n2abT95wVO7
=Gczh
-----END PGP SIGNATURE-----
--6749iDyxihQ87e9v--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20110918155649.GY1511>