Skip to content

Commit

Permalink
Fix process substitution infinite loops in virtual machines
Browse files Browse the repository at this point in the history
src/cmd/ksh93/sh/xec.c:
- An infinite loop may occur if an orphaned process from a process
  substitution isn't adopted by PID 1. Rather than close the FIFO
  when the parent process is init, check for the existence of the
  the parent shell or parent forked subshell by using kill(2).

src/cmd/ksh93/include/defs.h,
src/cmd/ksh93/sh/init.c,
src/cmd/ksh93/sh/subshell.c:
- Add a new 'parent_pid' variable to store the correct PPID. The
  PPID cannot be obtained after creating the process substitution
  (as that causes intermittent looping) and 'shgd->current_pid'
  can't be used as it's the process substitution's PID. As it's
  only used for FIFO process substitutions, hide it behind
  !SHOPT_DEVFD.

src/cmd/ksh93/tests/io.sh:
- Test for the infinite looping bug by using a forked
  command substitution.

Fixes: ksh93#213
  • Loading branch information
JohnoKing committed Mar 10, 2021
1 parent d4adc8f commit d91dda4
Show file tree
Hide file tree
Showing 6 changed files with 36 additions and 2 deletions.
4 changes: 4 additions & 0 deletions NEWS
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@ For full details, see the git log at: https://github.com/ksh93/ksh

Any uppercase BUG_* names are modernish shell bug IDs.

2021-03-10:
- Fixed an intermittent bug that caused process substitutions to infinitely
loop in Linux virtual machines that use systemd.

2021-03-09:

- The ${!foo@} and ${!foo*} expansions yield variable names beginning with foo,
Expand Down
3 changes: 3 additions & 0 deletions src/cmd/ksh93/include/defs.h
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,9 @@ struct shared
pid_t pid; /* $$, the main shell's PID (invariable) */
pid_t ppid; /* $PPID, the main shell's parent's PID */
pid_t current_pid; /* ${.sh.pid}, PID of current ksh process (updates when subshell forks) */
#if !SHOPT_DEVFD
pid_t parent_pid; /* PID of the parent shell or subshell (used in process substitutions) */
#endif
int realsubshell; /* ${.sh.subshell}, actual subshell level (including virtual and forked) */
unsigned char sigruntime[2];
Namval_t *bltin_nodes;
Expand Down
8 changes: 8 additions & 0 deletions src/cmd/ksh93/sh/init.c
Original file line number Diff line number Diff line change
Expand Up @@ -1238,7 +1238,11 @@ Shell_t *sh_init(register int argc,register char *argv[], Shinit_f userinit)
sh_regress_init(shp);
#endif
shgd = sh_newof(0,struct shared,1,0);
#if !SHOPT_DEVFD
shgd->parent_pid = shgd->current_pid = shgd->pid = getpid();
#else
shgd->current_pid = shgd->pid = getpid();
#endif
shgd->ppid = getppid();
shgd->userid=getuid();
shgd->euserid=geteuid();
Expand Down Expand Up @@ -1634,7 +1638,11 @@ int sh_reinit(char *argv[])
shp->errtrap = 0;
shp->end_fn = 0;
/* update ${.sh.pid}, $$, $PPID */
#if !SHOPT_DEVFD
shgd->parent_pid = shgd->current_pid = shgd->pid = getpid();
#else
shgd->current_pid = shgd->pid = getpid();
#endif
shgd->ppid = getppid();
return(1);
}
Expand Down
3 changes: 3 additions & 0 deletions src/cmd/ksh93/sh/subshell.c
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,9 @@ void sh_subfork(void)
sp->subpid=0;
shp->st.trapcom[0] = (comsub==2 ? NULL : trap);
shp->savesig = 0;
#if !SHOPT_DEVFD
shgd->parent_pid = shgd->current_pid;
#endif
/* sh_fork() increases ${.sh.subshell} but we forked an existing virtual subshell, so undo */
shgd->realsubshell--;
}
Expand Down
7 changes: 5 additions & 2 deletions src/cmd/ksh93/sh/xec.c
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,7 @@ struct funenv
static void fifo_check(void *handle)
{
Shell_t *shp = (Shell_t*)handle;
pid_t pid = getppid();
if(pid==1)
if(kill(shp->gd->parent_pid,0) < 0)
{
unlink(shp->fifo);
sh_done(shp,0);
Expand Down Expand Up @@ -1894,7 +1893,11 @@ int sh_exec(register const Shnode_t *t, int flags)
_sh_fork(shp,pid,0,0);
if(pid==0)
{
#if !SHOPT_DEVFD
shgd->parent_pid = shgd->current_pid = getpid();
#else
shgd->current_pid = getpid();
#endif
sh_exec(t->par.partre,flags);
shp->st.trapcom[0]=0;
sh_done(shp,0);
Expand Down
13 changes: 13 additions & 0 deletions src/cmd/ksh93/tests/io.sh
Original file line number Diff line number Diff line change
Expand Up @@ -715,5 +715,18 @@ got=$(export tmp; "$SHELL" -ec \
(redirect {v}>$tmp/v.out; echo ok2 >&$v) 2>/dev/null
[[ -r $tmp/v.out && $(<$tmp/v.out) == ok2 ]] || err_exit 'redirect {varname}>file not working in a subshell'
# ======
# Test for process substitution infinite looping
procsub_pid="$(
ulimit -t unlimited # fork the subshell
true >(true)
echo "$!"
)"
sleep 2 # wait for process to close (long wait to avoid false failures)
kill -0 $procsub_pid 2> /dev/null &&
kill -KILL $procsub_pid && # don't leave around what is effectively a zombie process
err_exit "process substitutions loop after parent shell finishes"
# ======
exit $((Errors<125?Errors:125))

0 comments on commit d91dda4

Please sign in to comment.