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 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.
> 
> 80KB seems quite a lot indeed, good to bring it down.
> 
> > 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
> [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 = 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) {
> 
> This sizeof is now the sizeof of a pointer. The comparison should be
> against FILENAME_MAX + 1 instead.
> 
> 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/localtime.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;
 
+       u = NULL;
+       res = -1;
        sp->goback = sp->goahead = FALSE;
 
        /* 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 = 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) >= 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 = 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 */
@@ -464,7 +483,7 @@ 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);
@@ -473,7 +492,7 @@ 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;
@@ -483,11 +502,11 @@ 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++;
@@ -512,7 +531,7 @@ 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) {
@@ -525,7 +544,7 @@ register const int  doextend;
                                ttisp->tt_ttisgmt = *p++;
                                if (ttisp->tt_ttisgmt != TRUE &&
                                        ttisp->tt_ttisgmt != 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] == '\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.
                */
@@ -570,39 +589,43 @@ 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)
@@ -620,7 +643,10 @@ register const int doextend;
                                        break;
                }
        }
-       return 0;
+       res = 0;
+out:
+       free(u);
+       return (res);
 }
 
 static int

Attachment: pgpctw5539hkn.pgp
Description: PGP signature

Reply via email to