From 4ce486a7a4daa5df3edfd2b23c250ceff7da0f3f Mon Sep 17 00:00:00 2001 From: Martijn Dekker Date: Thu, 24 Sep 2020 05:54:25 +0200 Subject: [PATCH] Fix hang in `comsubs` (rhbz#1062296) (re: 970069a6) The new command substitution mechanism imported in 970069a6 from Red Hat patches introduced this bug: backtick-style command substitutions hang when processing about 117KiB of data or more. It is fixed by another Red Hat patch: https://src.fedoraproject.org/rpms/ksh/blob/642af4d6/f/ksh-20140415-hokaido.patch It saves the value of the shp->comsub flag so that it is set to 2 (usually meaning new-style $(comsubs)) in two specific cases even when processing backtick comsubs. This stops the sh_subtmpfile() function in subshell.c from creating a /tmp file. However, I think that approach is quite ugly, so I'm taking a slightly different one that has the same effect. src/cmd/ksh93/include/defs.h, src/cmd/ksh93/sh/subshell.c: - Redefine sh_subtmpfile() to pass the comsub flag as an argument. (Remove the shp pointer argument, which is redundant; a pointer to the shell state can easily be obtained in the function.) src/cmd/ksh93/sh/xec.c: sh_exec(): - Apply the Red Hat fix by passing flag 2 to sh_subtmpfile(). src/cmd/ksh93/tests/subshell.sh: - Move regress test from ce68e1be from basic.sh to here; this is the place for command substitution tests as they are subshells. - Add regress test for this bug. All other changed files: - Update sh_subtmpfile() calls to pass on the shp->comsub flag. --- src/cmd/ksh93/include/defs.h | 4 ++-- src/cmd/ksh93/sh/args.c | 2 +- src/cmd/ksh93/sh/io.c | 4 ++-- src/cmd/ksh93/sh/subshell.c | 7 ++++--- src/cmd/ksh93/sh/xec.c | 10 ++++++---- src/cmd/ksh93/tests/basic.sh | 18 ------------------ src/cmd/ksh93/tests/subshell.sh | 27 +++++++++++++++++++++++++++ 7 files changed, 42 insertions(+), 30 deletions(-) diff --git a/src/cmd/ksh93/include/defs.h b/src/cmd/ksh93/include/defs.h index 7fc5160ff..8eab62100 100644 --- a/src/cmd/ksh93/include/defs.h +++ b/src/cmd/ksh93/include/defs.h @@ -211,7 +211,7 @@ struct shared unsigned char ignsig; /* ignored signal in subshell */ \ unsigned char lastsig; /* last signal received */ \ char pathinit; /* pathinit called from subshell */ \ - char comsub; /* set when in $() comsub */ \ + char comsub; /* set to 1 when in `` comsub, 2 when in $() comsub */ \ char subshare; /* set when in ${..} comsub */ \ char toomany; /* set when out of fd's */ \ char instance; /* in set_instance */ \ @@ -399,7 +399,7 @@ extern Namval_t *sh_scoped(Shell_t*, Namval_t*); extern Dt_t *sh_subfuntree(int); extern void sh_subjobcheck(pid_t); extern int sh_subsavefd(int); -extern void sh_subtmpfile(Shell_t*); +extern void sh_subtmpfile(char); extern char *sh_substitute(const char*,const char*,char*); extern void sh_timetraps(Shell_t*); extern const char *_sh_translate(const char*); diff --git a/src/cmd/ksh93/sh/args.c b/src/cmd/ksh93/sh/args.c index 96220d3e7..dbd2e251c 100644 --- a/src/cmd/ksh93/sh/args.c +++ b/src/cmd/ksh93/sh/args.c @@ -689,7 +689,7 @@ struct argnod *sh_argprocsub(Shell_t *shp,struct argnod *argp) ap->argflag &= ~ARG_RAW; fd = argp->argflag&ARG_RAW; if(fd==0 && shp->subshell) - sh_subtmpfile(shp); + sh_subtmpfile(shp->comsub); #if SHOPT_DEVFD sfwrite(shp->stk,e_devfdNN,8); pv[2] = 0; diff --git a/src/cmd/ksh93/sh/io.c b/src/cmd/ksh93/sh/io.c index ef9e880ec..b2346a49c 100644 --- a/src/cmd/ksh93/sh/io.c +++ b/src/cmd/ksh93/sh/io.c @@ -1141,7 +1141,7 @@ int sh_redirect(Shell_t *shp,struct ionod *iop, int flag) if(iof&IOPUT) ap->argflag = ARG_RAW; else if(shp->subshell) - sh_subtmpfile(shp); + sh_subtmpfile(shp->comsub); ap->argchn.ap = (struct argnod*)fname; ap = sh_argprocsub(shp,ap); fname = ap->argval; @@ -1203,7 +1203,7 @@ int sh_redirect(Shell_t *shp,struct ionod *iop, int flag) if(shp->subshell && dupfd==1) { if(sfset(sfstdout,0,0)&SF_STRING) - sh_subtmpfile(shp); + sh_subtmpfile(shp->comsub); if(shp->comsub==1) shp->subdup |= 1<jmplist; register struct subshell *sp = subshell_data->pipe; @@ -123,7 +124,7 @@ void sh_subtmpfile(Shell_t *shp) else if(errno!=EBADF) errormsg(SH_DICT,ERROR_system(1),e_toomany); /* popping a discipline forces a /tmp file create */ - if(shp->comsub != 1) + if(comsub_flag != 1) sfdisc(sfstdout,SF_POPDISC); if((fd=sffileno(sfstdout))<0) { @@ -181,7 +182,7 @@ void sh_subfork(void) trap = strdup(trap); /* see whether inside $(...) */ if(sp->pipe) - sh_subtmpfile(shp); + sh_subtmpfile(shp->comsub); shp->curenv = 0; shp->savesig = -1; if(pid = sh_fork(shp,FSHOWME,NIL(int*))) diff --git a/src/cmd/ksh93/sh/xec.c b/src/cmd/ksh93/sh/xec.c index 8d7c5a74a..862dabbf7 100644 --- a/src/cmd/ksh93/sh/xec.c +++ b/src/cmd/ksh93/sh/xec.c @@ -1319,7 +1319,7 @@ int sh_exec(register const Shnode_t *t, int flags) bp->notify = 0; bp->flags = (OPTIMIZE!=0); if(shp->subshell && nv_isattr(np,BLT_NOSFIO)) - sh_subtmpfile(shp); + sh_subtmpfile(shp->comsub); if(execflg && !shp->subshell && !shp->st.trapcom[0] && !shp->st.trap[SH_ERRTRAP] && shp->fn_depth==0 && !nv_isattr(np,BLT_ENV)) { @@ -1527,10 +1527,12 @@ int sh_exec(register const Shnode_t *t, int flags) int pipes[3]; if(shp->subshell) { - sh_subtmpfile(shp); + sh_subtmpfile(2); + if(shp->comsub==1 && (!(shp->fdstatus[1]&IONOSEEK))) + unpipe = iousepipe(shp); if((type&(FAMP|TFORK))==(FAMP|TFORK)) { - if(shp->comsub && !(shp->fdstatus[1]&IONOSEEK)) + if(shp->comsub && !(shp->fdstatus[1]&IONOSEEK) && !unpipe) unpipe = iousepipe(shp); sh_subfork(); } @@ -1909,7 +1911,7 @@ int sh_exec(register const Shnode_t *t, int flags) job.curjobid = 0; if(shp->subshell) { - sh_subtmpfile(shp); + sh_subtmpfile(2); if(shp->comsub==1 && !(shp->fdstatus[1]&IONOSEEK)) iousepipe(shp); } diff --git a/src/cmd/ksh93/tests/basic.sh b/src/cmd/ksh93/tests/basic.sh index 3e8427475..035206ab0 100755 --- a/src/cmd/ksh93/tests/basic.sh +++ b/src/cmd/ksh93/tests/basic.sh @@ -692,23 +692,5 @@ actual=$(exptest foo) [[ $actual == "$expect" ]] || err_exit 'Corruption of multibyte char following expansion of single-char name' \ "(expected $(printf %q "$expect"), got $(printf %q "$actual"))" -# ====== -# Crash in job handling code when running backtick-style command substitutions (rhbz#825520) -# The regression sometimes doesn't just crash, but freezes hard, so requires special handling. -cat >$tmp/backtick_crash.ksh <<'EOF' -binfalse=$(whence -p false) || exit -for ((i=0; i<250; i++)) -do test -z `"$binfalse" | "$binfalse" | /dev/null/nothing` -done -EOF -"$SHELL" -i "$tmp/backtick_crash.ksh" 2>/dev/null & # run test as bg job -test_pid=$! -(sleep 10; kill -s KILL "$test_pid" 2>/dev/null) & # another bg job to kill frozen test job -sleep_pid=$! -{ wait "$test_pid"; } 2>/dev/null # suppress any crash messages, which 'wait' would print -e=$? # get job's exit status from 'wait' -((!e)) || err_exit "backtick comsub crash/freeze (got status $e$( ((e>128)) && print -n / && kill -l "$e"))" -kill "$sleep_pid" 2>/dev/null - # ====== exit $((Errors<125?Errors:125)) diff --git a/src/cmd/ksh93/tests/subshell.sh b/src/cmd/ksh93/tests/subshell.sh index 802071376..2c9679945 100755 --- a/src/cmd/ksh93/tests/subshell.sh +++ b/src/cmd/ksh93/tests/subshell.sh @@ -794,5 +794,32 @@ actual=$(export bincat binecho; "$SHELL" 2>&1 -c \ [[ $actual == "$expect" ]] || err_exit 'Command substitution in pipeline fails (2)' \ "(expected $(printf %q "$expect"), got $(printf %q "$actual"))" +# ====== +# Crash in job handling code when running backtick-style command substitutions (rhbz#825520) +# The regression sometimes doesn't just crash, but freezes hard, so requires special handling. +cat >$tmp/backtick_crash.ksh <<'EOF' +binfalse=$(whence -p false) || exit +for ((i=0; i<250; i++)) +do test -z `"$binfalse" | "$binfalse" | /dev/null/nothing` +done +EOF +"$SHELL" -i "$tmp/backtick_crash.ksh" 2>/dev/null & # run test as bg job +test_pid=$! +(sleep 10; kill -s KILL "$test_pid" 2>/dev/null) & # another bg job to kill frozen test job +sleep_pid=$! +{ wait "$test_pid"; } 2>/dev/null # get job's exit status, suppressing signal messages +((!(e = $?))) || err_exit "backtick comsub crash/freeze (got status $e$( ((e>128)) && print -n / && kill -l "$e"))" +kill "$sleep_pid" 2>/dev/null + +# ====== +# Backtick command substitution hangs when filling out pipe buffer (rhbz#1062296) +"$SHELL" -c 'HANG=`dd if=/dev/zero bs=1k count=117 2>/dev/null`' & +test_pid=$! +(sleep 2; kill -s KILL "$test_pid" 2>/dev/null) & +sleep_pid=$! +{ wait "$test_pid"; } 2>/dev/null +((!(e = $?))) || err_exit "backtick comsub hang (got status $e$( ((e>128)) && print -n / && kill -l "$e"))" +kill "$sleep_pid" 2>/dev/null + # ====== exit $((Errors<125?Errors:125))