Fix crash while handling subshell trap (rhbz#1117404)

Contrary to the RH bug report, this is yet another bug with
virtual/non-forked subshells and has nothing to do with functions.
If a signal is ignored (empty trap) in the main shell while any
trap (empty or not) is set on the same signal in a subshell, a
crash eventually occurred upon restoring state when leaving the
subshell.

Original patch:
https://src.fedoraproject.org/rpms/ksh/blob/642af4d6/f/ksh-20120801-trapcom.patch

Prior discussion:
https://bugzilla.redhat.com/1117404

Paulo Andrade wrote there:
> The problem is that the sh_subshell function was saving pointers
> that could change, and when restoring, bad things would happen.
[...]
> The only comment I added:
> /* contents of shp->st.trapcom may change */
> may be a bit misleading, the "bad" save/restore already knows it,
> probably I should have added a better description telling that the
> data is, usually, modified in code like:
>
> tmp = buf[i]; buf[i] = strdup(tmp); free(tmp);
>
> so the shp->st.trapcom needs a "deep copy", as done in the
> patch, to properly save/restore pointers.

src/cmd/ksh93/sh/subshell.c,
src/cmd/ksh93/sh/xec.c:
- sh_subshell(), sh_funscope(): Make *savsig/*savstak into a
  **savsig array. Use strdup(3) to save the data and get known
  pointers that will not change. Free these upon restore.
- Change the comment from the patch as Paulo wished he had done.

src/cmd/ksh93/tests/subshell.sh:
- Test 2500 times. This should trigger the crash most of the time.

src/cmd/ksh93/sh/subshell.c

@@ -470,11 +470,11 @@ Sfio_t *sh_subshell(Shell_t *shp,Shnode_t *t, volatile int flags, int comsub)
 {
 	struct subshell sub_data;
 	register struct subshell *sp = &sub_data;
-	int jmpval,nsig=0,duped=0;
+	int jmpval,isig,nsig=0,duped=0;
 	unsigned int savecurenv = shp->curenv;
 	int savejobpgid = job.curpgid;
 	int *saveexitval = job.exitval;
-	char *savsig;
+	char **savsig;
 	Sfio_t *iop=0;
 	struct checkpt buff;
 	struct sh_scoped savst;
@@ -569,10 +569,24 @@ Sfio_t *sh_subshell(Shell_t *shp,Shnode_t *t, volatile int flags, int comsub)
 		/* save trap table */
 		shp->st.otrapcom = 0;
 		shp->st.otrap = savst.trap;
-		if((nsig=shp->st.trapmax*sizeof(char*))>0 || shp->st.trapcom[0])
+		if((nsig=shp->st.trapmax)>0 || shp->st.trapcom[0])
 		{
-			nsig += sizeof(char*);
-			memcpy(savsig=malloc(nsig),(char*)&shp->st.trapcom[0],nsig);
+			++nsig;
+			savsig = malloc(nsig * sizeof(char*));
+			/*
+			 * the data is, usually, modified in code like:
+			 *	tmp = buf[i]; buf[i] = strdup(tmp); free(tmp);
+			 * so shp->st.trapcom needs a "deep copy" to properly save/restore pointers.
+			 */
+			for (isig = 0; isig < nsig; ++isig)
+			{
+				if(shp->st.trapcom[isig] == Empty)
+					savsig[isig] = Empty;
+				else if(shp->st.trapcom[isig])
+					savsig[isig] = strdup(shp->st.trapcom[isig]);
+				else
+					savsig[isig] = NULL;
+			}
 			/* this nonsense needed for $(trap) */
 			shp->st.otrapcom = (char**)savsig;
 		}
@@ -732,7 +746,10 @@ Sfio_t *sh_subshell(Shell_t *shp,Shnode_t *t, volatile int flags, int comsub)
 		shp->st.otrap = 0;
 		if(nsig)
 		{
-			memcpy((char*)&shp->st.trapcom[0],savsig,nsig);
+			for (isig = 0; isig < nsig; ++isig)
+				if (shp->st.trapcom[isig] && shp->st.trapcom[isig]!=Empty)
+					free(shp->st.trapcom[isig]);
+			memcpy((char*)&shp->st.trapcom[0],savsig,nsig*sizeof(char*));
 			free((void*)savsig);
 		}
 		shp->options = sp->options;

src/cmd/ksh93/sh/xec.c

@@ -3042,10 +3042,10 @@ int sh_funscope(int argn, char *argv[],int(*fun)(void*),void *arg,int execflg)
 	struct dolnod		*argsav=0,*saveargfor;
 	struct sh_scoped	savst, *prevscope = shp->st.self;
 	struct argnod		*envlist=0;
-	int			jmpval;
+	int			isig,jmpval;
 	volatile int		r = 0;
 	int			n;
-	char 			*savstak;
+	char 			**savsig;
 	struct funenv		*fp = 0;
 	struct checkpt	*buffp = (struct checkpt*)stkalloc(shp->stk,sizeof(struct checkpt));
 	Namval_t		*nspace = shp->namespace;
@@ -3091,10 +3091,24 @@ int sh_funscope(int argn, char *argv[],int(*fun)(void*),void *arg,int execflg)
 	}
 	shp->st.cmdname = argv[0];
 	/* save trap table */
-	if((nsig=shp->st.trapmax*sizeof(char*))>0 || shp->st.trapcom[0])
-	{
-		nsig += sizeof(char*);
-		memcpy(savstak=stakalloc(nsig),(char*)&shp->st.trapcom[0],nsig);
+	if((nsig=shp->st.trapmax)>0 || shp->st.trapcom[0])
+	{
+		++nsig;
+		savsig = malloc(nsig * sizeof(char*));
+		/*
+		 * the data is, usually, modified in code like:
+		 *	tmp = buf[i]; buf[i] = strdup(tmp); free(tmp);
+		 * so shp->st.trapcom needs a "deep copy" to properly save/restore pointers.
+		 */
+		for (isig = 0; isig < nsig; ++isig)
+		{
+			if(shp->st.trapcom[isig] == Empty)
+				savsig[isig] = Empty;
+			else if(shp->st.trapcom[isig])
+				savsig[isig] = strdup(shp->st.trapcom[isig]);
+			else
+				savsig[isig] = NULL;
+		}
 	}
 	sh_sigreset(0);
 	argsav = sh_argnew(shp,argv,&saveargfor);
@@ -3158,10 +3172,14 @@ int sh_funscope(int argn, char *argv[],int(*fun)(void*),void *arg,int execflg)
 	shp->topscope = (Shscope_t*)prevscope;
 	nv_getval(sh_scoped(shp,IFSNOD));
 	if(nsig)
-		memcpy((char*)&shp->st.trapcom[0],savstak,nsig);
+	{
+		for (isig = 0; isig < nsig; ++isig)
+			if (shp->st.trapcom[isig] && shp->st.trapcom[isig]!=Empty)
+				free(shp->st.trapcom[isig]);
+		memcpy((char*)&shp->st.trapcom[0],savsig,nsig*sizeof(char*));
+		free((void*)savsig);
+	}
 	shp->trapnote=0;
-	if(nsig)
-		stakset(savstak,0);
 	shp->options = options;
 	shp->last_root = last_root;
 	if(jmpval == SH_JMPSUB)

src/cmd/ksh93/tests/subshell.sh

@@ -847,5 +847,19 @@ actual=${ get_value; }
 actual=`get_value`
 [[ $actual == "$expect" ]] || err_exit "\`Comsub\` failed to return output (expected '$expect', got '$actual')"
 
+# ======
+# https://bugzilla.redhat.com/1117404
+
+cat >$tmp/crash_rhbz1117404.ksh <<-'EOF'
+	trap "" HUP			# trigger part 1: signal ignored in main shell
+	i=0
+	for((i=0; i<2500; i++))
+	do	(trap ": foo" HUP)	# trigger part 2: any trap (empty or not) on same signal in subshell
+	done
+EOF
+got=$( { "$SHELL" "$tmp/crash_rhbz1117404.ksh"; } 2>&1)
+((!(e = $?))) || err_exit 'crash while handling subshell trap' \
+	"(got status $e$( ((e>128)) && print -n / && kill -l "$e"), $(printf %q "$got"))"
+
 # ======
 exit $((Errors<125?Errors:125))