Fix subshell leak for 3 special variables (re: 417883d, bd3e2a8)

McDutchie · McDutchie · commit e1c41bb2de3f · 2020-09-05T20:47:03.000+02:00
Using a process of elimination I've identified ${.sh.level} (SH_LEVELNOD) as the cause of the crash. This node apparently cannot be copied or moved without destabilising the shell. It contains the current depth of function calls and it cannot be changed by assignment, so this is not actually a problem. Meanwhile, this commit re-fixes it for the other three. src/cmd/ksh93/sh/subshell.c: - Simplify sh_assignok() by removing special-casing for L_ARGNOD, SH_SUBSCRNOD and SH_NAMENOD. 'add' now has 3 modes (0, 1, 2). - The test for a ${ subshare; } was actually wrong. sp->subshare is a saved backup value. We must test shp->subshare. (re: a9de50b) src/cmd/ksh93/bltins/typeset.c: - setall(): Update the mode 3 sh_assignok() call. src/cmd/ksh93/tests/variables.sh: - Regress-test subshell leaks for all special variables except ${.sh.level}.
diff --git a/src/cmd/ksh93/bltins/typeset.c b/src/cmd/ksh93/bltins/typeset.c
@@ -800,7 +800,7 @@ static int     setall(char **argv,register int flag,Dt_t *troot,struct tdata *tp
 			if (tp->aflag && (tp->argnum>0 || (curflag!=newflag)))
 			{
 				if(shp->subshell)
-					sh_assignok(np,3);
+					sh_assignok(np,2);
 				if(troot!=shp->var_tree)
 					nv_setattr(np,newflag&~NV_ASSIGN);
 				else
diff --git a/src/cmd/ksh93/sh/subshell.c b/src/cmd/ksh93/sh/subshell.c
@@ -235,8 +235,6 @@ int nv_subsaved(register Namval_t *np)
  * add == 1:    Create a copy of the node pointer in the current virtual subshell.
  * add == 2:    This will create a copy of the node pointer like 1, but it will disable the
  *              optimization for ${.sh.level}.
- * add == 3:    This is like 1, but it will never skip the following variables:
- *              ${.sh.level}, $_, ${.sh.subscript} and ${.sh.name}.
  */
 Namval_t *sh_assignok(register Namval_t *np,int add)
 {
@@ -248,11 +246,11 @@ Namval_t *sh_assignok(register Namval_t *np,int add)
 	Namval_t		*mpnext;
 	Namarr_t		*ap;
 	unsigned int		save;
-	/* don't bother to save if in a ${ subshare; } */
-	if(sp->subshare)
-		return(np);
-	/* don't bother with this */
-	if(!sp->shpwd || (add != 3 && ((add != 2 && np==SH_LEVELNOD) || np==L_ARGNOD || np==SH_SUBSCRNOD || np==SH_NAMENOD)))
+	/*
+	 * Don't save if told not to (see nv_restore()) or if we're in a ${ subshare; }.
+	 * Also, moving/copying ${.sh.level} (SH_LEVELNOD) may crash the shell.
+	 */
+	if(!sp->shpwd || shp->subshare || add<2 && np==SH_LEVELNOD)
 		return(np);
 	if((ap=nv_arrayptr(np)) && (mp=nv_opensub(np)))
 	{
diff --git a/src/cmd/ksh93/tests/variables.sh b/src/cmd/ksh93/tests/variables.sh
@@ -1011,6 +1011,23 @@ $SHELL -c '
 e=$?
 ((e == 1)) || err_exit "Failure in making one or more special variables readonly in a subshell (exit status $e)"
 
+# ... subshell leak test
+$SHELL -c '
+	errors=0
+	for var
+	do	if	[[ $var == .sh.level ]]
+		then	continue	# known to fail
+		fi
+		if	eval "($var=bug); [[ \${$var} == bug ]]" 2>/dev/null
+		then	echo "	$0: special variable $var leaks out of subshell" >&2
+			let errors++
+		fi
+	done
+	exit $((errors + 1))
+' subshell_leak_test "$@"
+e=$?
+((e == 1)) || err_exit "One or more special variables leak out of a subshell (exit status $e)"
+
 # ======
 # ${.sh.pid} should be the forked subshell's PID
 (

Original file line number	Diff line number	Diff line change
`@@ -800,7 +800,7 @@ static int setall(char *argv,register int flag,Dt_t troot,struct tdata *tp`
`800`	`800`	`if (tp->aflag && (tp->argnum>0 \|\| (curflag!=newflag)))`
`801`	`801`	`{`
`802`	`802`	`if(shp->subshell)`
`803`		`- sh_assignok(np,3);`
	`803`	`+ sh_assignok(np,2);`
`804`	`804`	`if(troot!=shp->var_tree)`
`805`	`805`	`nv_setattr(np,newflag&~NV_ASSIGN);`
`806`	`806`	`else`