]> git.pond.sub.org Git - empserver/commitdiff
accept: Resize thread stack to avoid stack smash for small worlds
authorMarkus Armbruster <armbru@pond.sub.org>
Mon, 6 Jan 2014 14:11:12 +0000 (15:11 +0100)
committerMarkus Armbruster <armbru@pond.sub.org>
Mon, 6 Jan 2014 19:50:06 +0000 (20:50 +0100)
I observed a stack overflow in news command on my x86_64 system
running Fedora 18.

Empire 2 settled on this formula for the stack size:

    stacksize = 100000
    /* budget */  + MAX(WORLD_SZ() * sizeof(int) * 7,
    /* power */ MAXNOC * sizeof(struct powstr));

Obviously attempts to provide space for known configuration-dependent
stack hogs.

The first hog is allegedly budget.  Bogus since day one: its large
arrays were static in Empire 2, and became dynamically allocated in
Empire 3.

The second one makes some sense: powe() has a struct powstr[MAXNOC].
It also has an int[MAXNOC], which isn't accounted for.

Except for ridiculously small worlds, the second term is smaller, and
only the (bogus) first term matters.

Two hogs are missing: head() has a struct histstr[MAXNOC][MAXNOC], and
news() has a short[MAXNOC][MAXNOC].  It also calls head().

I looked for more hogs with "gcc -fstack-usage", and found none.

On my x86_64 system, a news command needs almost 107KiB of stack.
Only slightly less when compiled for 32 bit.  Stack overrun for worlds
with fewer than some 320 sectors, thus unlikely to bite in real games.

Increase player stack size to 1 MiB.  Using MAXNOC to size the stack
isn't worth the trouble.

Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
src/lib/player/accept.c

index 7b7d111f9ebcf83311c2ec4ce84ed5259ec77dae..ab21861a39f3ec2315cdc796ad467a391da4ad64 100644 (file)
@@ -174,7 +174,6 @@ player_accept(void *unused)
     const char *p;
     int ns;
     int set = 1;
-    int stacksize;
     char buf[128];
 #ifdef RESOLVE_IPADDRESS
     struct hostent *hostp;
@@ -219,12 +218,8 @@ player_accept(void *unused)
        if (NULL != hostp)
            strcpy(np->hostname, hostp->h_name);
 #endif /* RESOLVE_IPADDRESS */
-       /* FIXME ancient black magic; figure out true stack need */
-       stacksize = 100000
-/* budget */  + MAX(WORLD_SZ() * sizeof(int) * 7,
-/* power */ MAXNOC * sizeof(struct powstr));
        sprintf(buf, "Conn%d", conn_cnt++);
-       empth_create(player_login, stacksize, 0, buf, np);
+       empth_create(player_login, 1024 * 1024, 0, buf, np);
     }
 }