ksh/ksh-1.0.11-subshell-trap-leak.patch
Vincent Mihalkovic 4fed517046 Fix trap and command-substitution memory leak
Resolves: RHEL-19580
2026-04-14 10:05:31 +02:00

280 lines
9.3 KiB
Diff

From: Martijn Dekker <martijn@inlv.org>
Date: Fri, 20 Mar 2026 16:01:40 +0000
Subject: Fix memleak invoking functions with trap set in parent scope (#948)
# Backport: ksh 1.0.10 + Patches 1-8; includes a0a56a43 + 4c391d08
# https://github.com/ksh93/ksh/pull/948
diff --git a/src/cmd/ksh93/bltins/trap.c b/src/cmd/ksh93/bltins/trap.c
index 5ddd446..d0d7416 100644
--- a/src/cmd/ksh93/bltins/trap.c
+++ b/src/cmd/ksh93/bltins/trap.c
@@ -154,20 +154,25 @@ int b_trap(int argc,char *argv[],Shbltin_t *context)
}
else
{
+ const int index = sig / 8;
+ const uint8_t sigbit = (uint8_t)1 << sig % 8;
/*
- * Trap or ignore a real signal. A virtual subshell needs to fork in
- * order to receive signals correctly and (because other commands
+ * Trap or ignore EXIT (0) or a signal. A virtual subshell must fork
+ * in order to receive signals correctly and (because other commands
* may cause a virtual subshell to fork) to ensure a persistent PID.
*/
- if(sh.subshell && !sh.subshare)
+ if(sig > 0 && sh.subshell && !sh.subshare)
sh_subfork();
if(sig >= sh.st.trapmax)
sh.st.trapmax = sig+1;
arg = sh.st.trapcom[sig];
sh_sigtrap(sig);
sh.st.trapcom[sig] = (sh.sigflag[sig]&SH_SIGOFF) ? Empty : sh_strdup(action);
- if(arg && arg != Empty)
+ /* free unless nofree bit is set */
+ if(arg && arg != Empty && !(sh.st.trapnofree[index] & sigbit))
free(arg);
+ /* clear nofree bit to avoid memory leak if trap is overwritten in same scope */
+ sh.st.trapnofree[index] &= ~sigbit;
}
}
/*
diff --git a/src/cmd/ksh93/include/shell.h b/src/cmd/ksh93/include/shell.h
index a7b090c..3c01a01 100644
--- a/src/cmd/ksh93/include/shell.h
+++ b/src/cmd/ksh93/include/shell.h
@@ -213,6 +213,7 @@ struct sh_scoped
char **trapcom; /* EXIT and signals */
char **otrapcom; /* save parent EXIT and signals for v=$(trap) */
struct Ufunction *real_fun; /* current 'function name' function */
+ uint8_t trapnofree[16]; /* bitmask to stop b_trap() freeing trapcom entries */
};
struct limits
diff --git a/src/cmd/ksh93/sh/subshell.c b/src/cmd/ksh93/sh/subshell.c
index a048445..ec384be 100644
--- a/src/cmd/ksh93/sh/subshell.c
+++ b/src/cmd/ksh93/sh/subshell.c
@@ -465,11 +465,12 @@ Sfio_t *sh_subshell(Shnode_t *t, volatile int flags, int comsub)
{
struct subshell sub_data;
struct subshell *sp = &sub_data;
- int jmpval,isig,nsig=0,fatalerror=0,saveerrno=0;
+ int n, jmpval, fatalerror = 0, saveerrno = 0;
unsigned int savecurenv = sh.curenv;
int savejobpgid = job.curpgid;
int *saveexitval = job.exitval;
char **savsig;
+ size_t nsig = 0;
Sfio_t *iop=0;
struct checkpt checkpoint;
struct sh_scoped savst;
@@ -533,7 +534,7 @@ Sfio_t *sh_subshell(Shnode_t *t, volatile int flags, int comsub)
}
if(sp->pwdfd<0)
{
- int n = open(e_dot,O_SEARCH);
+ n = sh_open(e_dot,O_SEARCH|O_cloexec);
if(n>=0)
{
sp->pwdfd = n;
@@ -553,33 +554,17 @@ Sfio_t *sh_subshell(Shnode_t *t, volatile int flags, int comsub)
sp->pwd = (sh.pwd?sh_strdup(sh.pwd):0);
sp->mask = sh.mask;
sh_stats(STAT_SUBSHELL);
- /* save trap table */
- sh.st.otrapcom = 0;
- sh.st.otrap = savst.trap;
- if((nsig=sh.st.trapmax)>0 || sh.st.trapcom[0])
- {
- savsig = sh_malloc(nsig * sizeof(char*));
- /*
- * the data is, usually, modified in code like:
- * tmp = buf[i]; buf[i] = sh_strdup(tmp); free(tmp);
- * so sh.st.trapcom needs a "deep copy" to properly save/restore pointers.
- */
- for (isig = 0; isig < nsig; ++isig)
- {
- if(sh.st.trapcom[isig] == Empty)
- savsig[isig] = Empty;
- else if(sh.st.trapcom[isig])
- savsig[isig] = sh_strdup(sh.st.trapcom[isig]);
- else
- savsig[isig] = NULL;
- }
- /* this is needed for var=$(trap) */
- sh.st.otrapcom = (char**)savsig;
- }
sp->cpid = sh.cpid;
sp->coutpipe = sh.coutpipe;
sp->cpipe = sh.cpipe[1];
sh.cpid = 0;
+ /* save trap table */
+ memset(sh.st.trapnofree, 0xFF, sizeof sh.st.trapnofree);
+ sh.st.otrap = savst.trap;
+ if((nsig = sh.st.trapmax * sizeof(char**)) > 0)
+ savsig = sh.st.otrapcom = memcpy(stkalloc(sh.stk, nsig), sh.st.trapcom, nsig);
+ else
+ sh.st.otrapcom = NULL;
if(sh_isoption(SH_FUNCTRACE) && sh.st.trap[SH_DEBUGTRAP] && *sh.st.trap[SH_DEBUGTRAP])
save_debugtrap = sh_strdup(sh.st.trap[SH_DEBUGTRAP]);
sh_sigreset(0);
@@ -756,7 +741,6 @@ Sfio_t *sh_subshell(Shnode_t *t, volatile int flags, int comsub)
sh.bckpid = sp->bckpid;
if(!sh.subshare) /* restore environment if saved */
{
- int n;
struct rand *rp;
sh.options = sp->options;
/* Clean up subshell hash table. */
@@ -812,13 +796,7 @@ Sfio_t *sh_subshell(Shnode_t *t, volatile int flags, int comsub)
sh.st = savst;
sh.st.otrap = 0;
if(nsig)
- {
- for (isig = 0; isig < nsig; ++isig)
- if (sh.st.trapcom[isig] && sh.st.trapcom[isig]!=Empty)
- free(sh.st.trapcom[isig]);
- memcpy((char*)&sh.st.trapcom[0],savsig,nsig*sizeof(char*));
- free(savsig);
- }
+ memcpy(sh.st.trapcom, savsig, nsig);
sh.options = sp->options;
/* restore the present working directory */
#if _lib_fchdir
@@ -883,10 +861,10 @@ Sfio_t *sh_subshell(Shnode_t *t, volatile int flags, int comsub)
}
sh_sigcheck();
sh.trapnote = 0;
- nsig = sh.savesig;
+ n = sh.savesig;
sh.savesig = 0;
- if(nsig>0)
- kill(sh.current_pid,nsig);
+ if(n > 0)
+ kill(sh.current_pid, n);
if(sp->subpid)
job_wait(sp->subpid);
sh.comsub = sp->comsub;
diff --git a/src/cmd/ksh93/sh/xec.c b/src/cmd/ksh93/sh/xec.c
index 8f82792..e4da79f 100644
--- a/src/cmd/ksh93/sh/xec.c
+++ b/src/cmd/ksh93/sh/xec.c
@@ -1747,22 +1747,21 @@ int sh_exec(const Shnode_t *t, int flags)
if(!sh.subshell && !sh.st.trapdontexec && (flags&sh_state(SH_NOFORK)))
{
- /* This is the last command, so avoid creating a subshell */
- char *savsig;
- int nsig,jmpval;
+ /* This is the last command, so avoid creating a subshell, but still act like one */
+ size_t nsig;
+ int jmpval;
struct checkpt *buffp = stkalloc(sh.stk,sizeof(struct checkpt));
- sh.st.otrapcom = 0;
- if((nsig=sh.st.trapmax*sizeof(char*))>0 || sh.st.trapcom[0])
- {
- nsig += sizeof(char*);
- savsig = sh_malloc(nsig);
- memcpy(savsig,(char*)&sh.st.trapcom[0],nsig);
- sh.st.otrapcom = (char**)savsig;
- }
- /* Still act like a subshell: reseed $RANDOM and increment ${.sh.subshell} */
+ /* Save traps for printing, then reset them */
+ memset(sh.st.trapnofree, 0xFF, sizeof sh.st.trapnofree);
+ if ((nsig = sh.st.trapmax * sizeof(char*)) > 0)
+ sh.st.otrapcom = memcpy(stkalloc(sh.stk, nsig), sh.st.trapcom, nsig);
+ else
+ sh.st.otrapcom = NULL;
+ sh_sigreset(0);
+ /* Reseed $RANDOM and increment ${.sh.subshell} */
sh_invalidate_rand_seed();
sh.realsubshell++;
- sh_sigreset(0);
+ /* Execute the last command and exit normally, except for SH_JMPSCRIPT */
sh_pushcontext(buffp,SH_JMPEXIT);
jmpval = sigsetjmp(buffp->buff,0);
if(jmpval==0)
sh_exec(t->par.partre,flags);
@@ -2947,16 +2946,16 @@
int sh_funscope(int argn, char *argv[],int(*fun)(void*),void *arg,int execflg)
{
char *trap;
- int nsig;
struct dolnod *argsav=0,*saveargfor;
struct sh_scoped *savst = stkalloc(sh.stk,sizeof(struct sh_scoped));
struct sh_scoped *prevscope = sh.st.self;
struct argnod *envlist=0;
- int isig,jmpval;
+ int jmpval;
volatile int r = 0;
int n;
char save_invoc_local;
char **savsig, *save_debugtrap = 0;
+ size_t nsig;
struct funenv *fp = 0;
struct checkpt *buffp = stkalloc(sh.stk,sizeof(struct checkpt));
Namval_t *nspace = sh.namespace;
@@ -3000,24 +2999,9 @@
}
sh.st.cmdname = argv[0];
/* save trap table */
- if((nsig=sh.st.trapmax)>0 || sh.st.trapcom[0])
- {
- savsig = sh_malloc(nsig * sizeof(char*));
- /*
- * the data is, usually, modified in code like:
- * tmp = buf[i]; buf[i] = sh_strdup(tmp); free(tmp);
- * so sh.st.trapcom needs a "deep copy" to properly save/restore pointers.
- */
- for (isig = 0; isig < nsig; ++isig)
- {
- if(sh.st.trapcom[isig] == Empty)
- savsig[isig] = Empty;
- else if(sh.st.trapcom[isig])
- savsig[isig] = sh_strdup(sh.st.trapcom[isig]);
- else
- savsig[isig] = NULL;
- }
- }
+ memset(sh.st.trapnofree, 0xFF, sizeof sh.st.trapnofree);
+ if((nsig = sh.st.trapmax * sizeof(char**)) > 0)
+ savsig = memcpy(stkalloc(sh.stk, nsig), sh.st.trapcom, nsig);
if(!fun && sh_isoption(SH_FUNCTRACE) && sh.st.trap[SH_DEBUGTRAP] && *sh.st.trap[SH_DEBUGTRAP])
save_debugtrap = sh_strdup(sh.st.trap[SH_DEBUGTRAP]);
sh_sigreset(-1);
@@ -3092,13 +3076,7 @@
sh.topscope = (Shscope_t*)prevscope;
nv_getval(sh_scoped(IFSNOD));
if(nsig)
- {
- for (isig = 0; isig < nsig; ++isig)
- if (sh.st.trapcom[isig] && sh.st.trapcom[isig]!=Empty)
- free(sh.st.trapcom[isig]);
- memcpy((char*)&sh.st.trapcom[0],savsig,nsig*sizeof(char*));
- free(savsig);
- }
+ memcpy(sh.st.trapcom, savsig, nsig);
sh.trapnote=0;
sh.options = options;
sh.last_root = last_root;
diff --git a/src/cmd/ksh93/tests/leaks.sh b/src/cmd/ksh93/tests/leaks.sh
index ec882bb..c2e57e8 100755
--- a/src/cmd/ksh93/tests/leaks.sh
+++ b/src/cmd/ksh93/tests/leaks.sh
@@ -423,5 +423,15 @@ DO
unset baz
DONE
+# ======
+TEST title='trap and command substitution'
+ trap ': long-enough trap action to detect the leak' USR1
+DO
+ v=`echo a`
+ v=$(echo a)
+ (echo a)
+DONE >/dev/null
+trap - USR1
+
# ======
exit $((Errors<125?Errors:125))