From fe20311fe9ca9fbc69df84d8cae3aef31e4fa740 Mon Sep 17 00:00:00 2001 From: Martijn Dekker Date: Sun, 20 Sep 2020 23:58:44 +0200 Subject: [PATCH] Fix `command substitution` memory leaks (rhbz#982142) This fixes two memory leaks in old-style command substitutions (one when invoking an alias, one when invoking an autoloaded function), as well as a possible third leak with an unknown reproducer, by applying this Red Hat patch: https://src.fedoraproject.org/rpms/ksh/blob/642af4d6/f/ksh-20120801-mlikfiks.patch src/cmd/ksh93/sh/macro.c: comsubst(): - For as-yet unknown reasons, the alias leak did not occur when adding a space at the end of the command substitution, as in a=`some_alias `. This fix is a workaround that simply writes an extra space to the stack. TODO: a real fix. src/cmd/ksh93/sh/path.c: funload(): - Add missing free() before return. This fixes the leak with autoloaded functions. src/cmd/ksh93/sh/lex.c: alias_exceptf(): - This function is called "whenever an end of string is found with alias". This adds a check for an SF_FINAL stream status flag when deciding whether to call free(). In sfio.h this is commented as: #define SF_FINAL 11 /* closing is done except stream free */ When I revert this change, none of the regression tests fail, so I don't know how to trigger this supposed leak. But it makes some sense given the sfio.h comment, so I'll keep it. src/cmd/ksh93/tests/leaks.sh: - Add the reproducers from rhbz#982142 as regression tests (including an extra one for nested command substitutions that was already fixed as of 93u+, but testing is good). I replaced the external 'expr' and 'ls' commands by uses of the 'true' builtin, otherwise the tests take far too long to run with 16384 iterations. At least the alias leak was still behaving identically after replacing 'ls' by 'true'. --- src/cmd/ksh93/sh/lex.c | 2 +- src/cmd/ksh93/sh/macro.c | 1 + src/cmd/ksh93/sh/path.c | 1 + src/cmd/ksh93/tests/leaks.sh | 40 ++++++++++++++++++++++++++++++++++++ 4 files changed, 43 insertions(+), 1 deletion(-) diff --git a/src/cmd/ksh93/sh/lex.c b/src/cmd/ksh93/sh/lex.c index b9e7ef48020b..d5baf5d0a983 100644 --- a/src/cmd/ksh93/sh/lex.c +++ b/src/cmd/ksh93/sh/lex.c @@ -2445,7 +2445,7 @@ static int alias_exceptf(Sfio_t *iop,int type,Sfdisc_t *handle) if(dp!=handle) sfdisc(iop,dp); } - else if(type==SF_FINAL) + else if(type==SF_DPOP || type==SF_FINAL) free((void*)ap); goto done; } diff --git a/src/cmd/ksh93/sh/macro.c b/src/cmd/ksh93/sh/macro.c index 5605a21bcd09..7af9ca1dc266 100644 --- a/src/cmd/ksh93/sh/macro.c +++ b/src/cmd/ksh93/sh/macro.c @@ -2110,6 +2110,7 @@ static void comsubst(Mac_t *mp,register Shnode_t* t, int type) } sfputc(stkp,c); } + sfputc(stkp,' '); /* rhbz#982142: a=`some_alias` leaked memory, a=`some_alias ` did not! TODO: non-hack fix */ c = stktell(stkp); str=stkfreeze(stkp,1); /* disable verbose and don't save in history file */ diff --git a/src/cmd/ksh93/sh/path.c b/src/cmd/ksh93/sh/path.c index 7137ebab29ce..032871479015 100644 --- a/src/cmd/ksh93/sh/path.c +++ b/src/cmd/ksh93/sh/path.c @@ -612,6 +612,7 @@ static void funload(Shell_t *shp,int fno, const char *name) } while((rp=dtnext(shp->fpathdict,rp)) && strcmp(pname,rp->fname)==0); sh_close(fno); + free((void*)pname); return; } sh_onstate(SH_NOLOG); diff --git a/src/cmd/ksh93/tests/leaks.sh b/src/cmd/ksh93/tests/leaks.sh index b29b07355b39..40d815ccd65f 100755 --- a/src/cmd/ksh93/tests/leaks.sh +++ b/src/cmd/ksh93/tests/leaks.sh @@ -270,5 +270,45 @@ done after=$(getmem) err_exit_if_leak 'unalias' +# ====== +# Red Hat bug rhbz#982142: command substitution leaks + +# case1: Nested command substitutions +# (reportedly already fixed in 93u+, but let's keep the test) +before=$(getmem) +for ((i=0; i < N; i++)) +do a=`true 1 + \`true 1 + 1\`` # was: a=`expr 1 + \`expr 1 + 1\`` +done +after=$(getmem) +err_exit_if_leak 'nested command substitutions' + +# case2: Command alias +alias ls='true -ltr' # was: alias ls='ls -ltr' +before=$(getmem) +for ((i=0; i < N; i++)) +do eval 'a=`ls`' +done +after=$(getmem) +unalias ls +err_exit_if_leak 'alias in command substitution' + +# case3: Function call via autoload +cat >$tmp/func1 <<\EOF +function func1 +{ + echo "func1 call"; +} +EOF +FPATH=$tmp +autoload func1 +before=$(getmem) +for ((i=0; i < N; i++)) +do a=`func1` +done +after=$(getmem) +unset -f func1 +unset -v FPATH +err_exit_if_leak 'function call via autoload in command substitution' + # ====== exit $((Errors<125?Errors:125))