From 6304dfce41a991ec9d6d68fba5cc64e16eecc7b5 Mon Sep 17 00:00:00 2001 From: Martijn Dekker Date: Fri, 11 Feb 2022 02:21:47 +0000 Subject: [PATCH] Fix corner-case >&- redirection leak out of subshell Reproducer: exec 9>&1 ( { exec 9>&1; } 9>&- ) echo "test" >&9 # => 9: cannot open [Bad file descriptor] The 9>&- incorrectly persists beyond the { } block that it was attached to *and* beyond the ( ) subshell. This is yet another bug with non-forking subshells; forking it with something like 'ulimit -t unlimited' works around the bug. In over a year we have not been able to find a real fix, but I came up with a workaround that forks a virtual subshell whenever it executes a code block with a >&- or <&- redirection attached. That use case is obscure enough that it should not cause any performance regression except in very rare corner cases. src/cmd/ksh93/sh/xec.c: sh_exec(): TSETIO: - This is where redirections attached to code blocks are handled. Check for a >&- or <&- redirection using bit flaggery from shnodes.h and fork if we're executing such in a virtual subshell. Resolves: https://github.com/ksh93/ksh/issues/161 Thanks to @ko1nksm for the bug report. --- NEWS | 5 +++++ src/cmd/ksh93/include/version.h | 2 +- src/cmd/ksh93/sh/xec.c | 16 ++++++++++++++++ src/cmd/ksh93/tests/io.sh | 32 +++++++++++++++++++++++++++++++- 4 files changed, 53 insertions(+), 2 deletions(-) diff --git a/NEWS b/NEWS index 2f1ec7b5b..6e718db0c 100644 --- a/NEWS +++ b/NEWS @@ -3,6 +3,11 @@ For full details, see the git log at: https://github.com/ksh93/ksh/tree/1.0 Any uppercase BUG_* names are modernish shell bug IDs. +2022-02-11: + +- Fixed a bug in which a >&- or <&- redirection could persist past a + subshell in certain corner cases involving 'exec' or 'redirect'. + 2022-02-08: - Multiple bugs were fixed in the method that ksh uses to execute a shell diff --git a/src/cmd/ksh93/include/version.h b/src/cmd/ksh93/include/version.h index b889ee203..aed2f3a41 100644 --- a/src/cmd/ksh93/include/version.h +++ b/src/cmd/ksh93/include/version.h @@ -21,7 +21,7 @@ #define SH_RELEASE_FORK "93u+m" /* only change if you develop a new ksh93 fork */ #define SH_RELEASE_SVER "1.0.0-beta.2" /* semantic version number: https://semver.org */ -#define SH_RELEASE_DATE "2022-02-08" /* must be in this format for $((.sh.version)) */ +#define SH_RELEASE_DATE "2022-02-11" /* must be in this format for $((.sh.version)) */ #define SH_RELEASE_CPYR "(c) 2020-2022 Contributors to ksh " SH_RELEASE_FORK /* Scripts sometimes field-split ${.sh.version}, so don't change amount of whitespace. */ diff --git a/src/cmd/ksh93/sh/xec.c b/src/cmd/ksh93/sh/xec.c index 7c6727de1..ec90cee5a 100644 --- a/src/cmd/ksh93/sh/xec.c +++ b/src/cmd/ksh93/sh/xec.c @@ -1821,6 +1821,22 @@ int sh_exec(register const Shnode_t *t, int flags) int jmpval, waitall = 0; int simple = (t->fork.forktre->tre.tretyp&COMMSK)==TCOM; struct checkpt *buffp = (struct checkpt*)stkalloc(sh.stk,sizeof(struct checkpt)); + if(sh.subshell && !sh.subshare && t->fork.forkio) + { + /* Subshell forking workaround for https://github.com/ksh93/ksh/issues/161 + * Check each redirection for >&- or <&- + * TODO: find the elusive real fix */ + struct ionod *i = t->fork.forkio; + do + { + if((i->iofile & ~(IOUFD|IOPUT)) == (IOMOV|IORAW) && !strcmp(i->ioname,"-")) + { + sh_subfork(); + break; + } + } + while(i = i->ionxt); + } sh_pushcontext(buffp,SH_JMPIO); if(type&FPIN) { diff --git a/src/cmd/ksh93/tests/io.sh b/src/cmd/ksh93/tests/io.sh index 302637827..739dee7bc 100755 --- a/src/cmd/ksh93/tests/io.sh +++ b/src/cmd/ksh93/tests/io.sh @@ -766,7 +766,7 @@ got=$(export tmp; "$SHELL" -ec \ # ====== # Redirections of the form {varname}>file stopped working if brace expansion was turned off ((SHOPT_BRACEPAT)) && set +B -{ redirect {v}>$tmp/v.out && echo ok >&$v; } 2>/dev/null && redirect {v}>&- +(redirect {v}>$tmp/v.out && echo ok >&$v && redirect {v}>&-) 2>/dev/null ((SHOPT_BRACEPAT)) && set -B [[ -r $tmp/v.out && $(<$tmp/v.out) == ok ]] || err_exit '{varname}>file not working with brace expansion turned off' @@ -924,5 +924,35 @@ then kill "$!" # the sleep process else err_exit "comsub hangs after fork with stdout redirection" fi +# ====== +# https://github.com/ksh93/ksh/issues/161 +got=$( + set +x + redirect 2>&1 9>&1 + ( { redirect 9>&1; } 6<&2 9<&- ) + echo "test" >&9 # => 9: cannot open [Bad file descriptor] +) +[[ $got == 'test' ]] || err_exit "File descriptor is unexpectedly closed after exec in subshell" \ + "(expected 'test', got $(printf %q "$got"))" +got=$( + set +x + exec 2>&1 9>&1 + exec 9>&- + v=$( { exec 9>&1; } ) + echo "test" >&9 +) +exp=': 9: cannot open [' +[[ $got == *"$exp"* ]] || err_exit "issue 161 hypothetical bug 1" \ + "(expected match of *$(printf %q "$exp")*, got $(printf %q "$got"))" +got=$( + set +x + exec 2>&1 + openfd() { exec 6>&1; } + openfd + echo "test" >&6 +) +[[ $got == 'test' ]] || err_exit "issue 161 hypothetical bug 2" \ + "(expected 'test', got $(printf %q "$got"))" + # ====== exit $((Errors<125?Errors:125))