ash: [EXPAND] Fix ifsfirst/ifslastp leak
authorDenys Vlasenko <vda.linux@googlemail.com>
Thu, 27 Oct 2016 12:46:50 +0000 (14:46 +0200)
committerDenys Vlasenko <vda.linux@googlemail.com>
Thu, 27 Oct 2016 12:46:50 +0000 (14:46 +0200)
Upstream commit:

    Date: Wed, 8 Sep 2010 20:07:26 +0800
    [EXPAND] Fix ifsfirst/ifslastp leak

    As it stands expandarg may return with a non-NULL ifslastp which
    then confuses any subsequent ifsbreakup user that doesn't clear
    it directly.

    What's worse, if we get interrupted before we hit ifsfree in
    expandarg we will leak memory.

    This patch fixes this by always calling ifsfree in expandarg
    thus ensuring that ifslastp is always NULL on the normal path.
    It also adds an ifsfree call to the RESET path to ensure that
    memory isn't leaked.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Fallout 1:

    Date: Mon, 18 Oct 2010 10:55:42 +0800
    [EXPAND] Fix ifsfirst/ifslastp leak in casematch

    The commit f42e443bb511ed3224f09b4fcf0772438ebdbbfa

        [EXPAND] Fix ifsfirst/ifslastp leak

    revealed yet another ifsfirst/ifslastp leak in casematch.
    Previously it was hidden because ifsfirst/ifslastp was cleared
    unconditionally on entry (which caused the leakage of those
    entries).

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Fallout 2:

    Date: Sun, 28 Nov 2010 21:09:51 +0800
    [EXPAND] Free IFS state in evalbackcmd

    On Sun, Nov 07, 2010 at 04:04:20PM -0600, Jonathan Nieder wrote:
    > Herbert Xu wrote:
    > > commit f42e443bb511ed3224f09b4fcf0772438ebdbbfa
    > > Author: Herbert Xu <herbert@gondor.apana.org.au>
    > > Date:   Wed Sep 8 20:07:26 2010 +0800
    > >
    > >     [EXPAND] Fix ifsfirst/ifslastp leak
    >
    > Another puzzle bisecting to f42e443bb.  This one comes from the
    > grub-mkconfig script:
    >
    >  $ sh -c 'datadir=/usr/share; pkgdatadir=${datadir}/`cat`' 2>&1 | cat -A
    >  cat: M-^\^M^F^HM-4^M^F^HM-(^M^F^H: No such file or directory$
    >  cat: M-(^M^F^H: No such file or directory$
    >
    > Still reproducible with 016b529.  I'll try to find time to look into
    > it, but thought you might like to know nevertheless.

    This is the symptom of another leak.  In this case evalbackcmd
    occurs in the middle of an expansion (as it should) but the forked
    child never clears the previous IFS state.

    This patch adds the missing ifsfree call.

    This wasn't as much of a problem as the previously discovered leaks
    since all it means is that the child gets to carry around the parent's
    expansion state and the child is usually short-lived.

Reported-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Fallout 3:

    Date: Tue, 15 Mar 2011 16:01:34 +0800
    [EXPAND] Free IFS state after here document expansion

    Here's another bug bisecting to f42e443bb ([EXPAND] Fix
    ifsfirst/ifslastp leak, 2010-09-08).  It was found with the following
    test case, based on the configure script for Tracker:

        dash -x -c '
                <<-_ACEOF
                $@
                _ACEOF
                exec
        ' - abcdefgh
        +
        + exec   ?a
        exec: 1: : Permission denied

    The missing ifsfree call is in expandarg when it returns to openhere
    during here document expansion.

Reported-by: Aurelien Jarno <aurel32@debian.org>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
function                                             old     new   delta
ifsfree                                                -      66     +66
ash_main                                            1490    1495      +5
argstr                                              1154    1159      +5
evalcase                                             275     270      -5
expandarg                                            972     888     -84
------------------------------------------------------------------------------
(add/remove: 1/0 grow/shrink: 2/2 up/down: 76/-89)            Total: -13 bytes

Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>
shell/ash.c

index 7acb33e7e4620d458ab54195d3363ae9646f450c..d3837841142e297b8f00b0c34768148b424661ea 100644 (file)
@@ -5663,19 +5663,22 @@ ifsbreakup(char *string, struct arglist *arglist)
 static void
 ifsfree(void)
 {
-       struct ifsregion *p;
+       struct ifsregion *p = ifsfirst.next;
+
+       if (!p)
+               goto out;
 
        INT_OFF;
-       p = ifsfirst.next;
        do {
                struct ifsregion *ifsp;
                ifsp = p->next;
                free(p);
                p = ifsp;
        } while (p);
-       ifslastp = NULL;
        ifsfirst.next = NULL;
        INT_ON;
+ out:
+       ifslastp = NULL;
 }
 
 static size_t
@@ -5989,6 +5992,7 @@ evalbackcmd(union node *n, struct backcmd *result)
  * For now, preserve bash-like behavior, it seems to be somewhat more useful:
  */
                eflag = 0;
+               ifsfree();
                evaltree(n, EV_EXIT); /* actually evaltreenr... */
                /* NOTREACHED */
        }
@@ -7296,15 +7300,14 @@ expandarg(union node *arg, struct arglist *arglist, int flag)
 
        argbackq = arg->narg.backquote;
        STARTSTACKSTR(expdest);
-       ifsfirst.next = NULL;
-       ifslastp = NULL;
        TRACE(("expandarg: argstr('%s',flags:%x)\n", arg->narg.text, flag));
        argstr(arg->narg.text, flag,
                        /* var_str_list: */ arglist ? arglist->list : NULL);
        p = _STPUTC('\0', expdest);
        expdest = p - 1;
        if (arglist == NULL) {
-               return;                 /* here document expanded */
+               /* here document expanded */
+               goto out;
        }
        p = grabstackstr(p);
        TRACE(("expandarg: p:'%s'\n", p));
@@ -7327,13 +7330,14 @@ expandarg(union node *arg, struct arglist *arglist, int flag)
                *exparg.lastp = sp;
                exparg.lastp = &sp->next;
        }
-       if (ifsfirst.next)
-               ifsfree();
        *exparg.lastp = NULL;
        if (exparg.list) {
                *arglist->lastp = exparg.list;
                arglist->lastp = exparg.lastp;
        }
+
+ out:
+       ifsfree();
 }
 
 /*
@@ -7367,10 +7371,10 @@ casematch(union node *pattern, char *val)
        setstackmark(&smark);
        argbackq = pattern->narg.backquote;
        STARTSTACKSTR(expdest);
-       ifslastp = NULL;
        argstr(pattern->narg.text, EXP_TILDE | EXP_CASE,
                        /* var_str_list: */ NULL);
        STACKSTRNUL(expdest);
+       ifsfree();
        result = patmatch(stackblock(), val);
        popstackmark(&smark);
        return result;
@@ -13293,6 +13297,7 @@ read_profile(const char *name)
 /*
  * This routine is called when an error or an interrupt occurs in an
  * interactive shell and control is returned to the main command loop.
+ * (In dash, this function is auto-generated by build machinery).
  */
 static void
 reset(void)
@@ -13300,13 +13305,19 @@ reset(void)
        /* from eval.c: */
        evalskip = 0;
        loopnest = 0;
+
+       /* from expand.c: */
+       ifsfree();
+
        /* from input.c: */
        g_parsefile->left_in_buffer = 0;
        g_parsefile->left_in_line = 0;      /* clear input buffer */
        popallfiles();
+
        /* from parser.c: */
        tokpushback = 0;
        checkkwd = 0;
+
        /* from redir.c: */
        while (redirlist)
                popredir(/*drop:*/ 0, /*restore:*/ 0);