Skip to content

Commit

Permalink
Fix subshell leak for 3 special variables (re: 417883d, bd3e2a8)
Browse files Browse the repository at this point in the history
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}.
  • Loading branch information
McDutchie committed Sep 5, 2020
1 parent 417883d commit e1c41bb
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 8 deletions.
2 changes: 1 addition & 1 deletion src/cmd/ksh93/bltins/typeset.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 5 additions & 7 deletions src/cmd/ksh93/sh/subshell.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand All @@ -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)))
{
Expand Down
17 changes: 17 additions & 0 deletions src/cmd/ksh93/tests/variables.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
(
Expand Down

0 comments on commit e1c41bb

Please sign in to comment.