From 91a7c2e3e9feb8ac1391146ebcda9e6adfcd3cfb Mon Sep 17 00:00:00 2001 From: Martijn Dekker Date: Mon, 27 Dec 2021 03:00:15 +0000 Subject: [PATCH] Fix crash/freeze upon interrupting command substitution with pipe On some systems (at least Linux and macOS): 1. Run on a command line: t=$(sleep 10|while :; do :; done) 2. Press Ctrl+C in the first 10 seconds. 3. Execute any other command substitution. The shell crashes. Analysis: Something in the job_wait() call in the sh_subshell() restore routine may be interrupted by a signal such as SIGINT on Linux and macOS. Exactly what that interruptible thing is remains to be determined. In any case, since job_wait() was invoked after sh_popcontext(), interrupting it caused the sh_subshell() restore routine to be aborted, resulting in an inconsistent state of the shell. The fix is to sh_popcontext() at a later stage instead. src/cmd/ksh93/sh/subshell.c: sh_subshell(): - Rename struct checkpt buff to checkpoint because it's clearer. - Move the sh_popcontext() call to near the end, just after decreasing the subshell level counters and restoring the global subshell data struct to its parent. This seems like a logical place for it and could allow other things to be interrupted, too. - Get rid of the if(shp->subshell) because it is known that the value is > 0 at this point. - The short exit routine run if the subshell forked now needs a new sh_popcontext() call, because this is handled before restoring the virtual subshell state. - While we're here, do a little more detransitioning from all those pointless shp pointers. Fixes: https://github.com/ksh93/ksh/issues/397 --- NEWS | 5 +++++ src/cmd/ksh93/include/version.h | 2 +- src/cmd/ksh93/sh/subshell.c | 22 ++++++++++------------ 3 files changed, 16 insertions(+), 13 deletions(-) diff --git a/NEWS b/NEWS index 34df220ff..7cb844e8f 100644 --- a/NEWS +++ b/NEWS @@ -3,6 +3,11 @@ For full details, see the git log at: https://github.com/ksh93/ksh Any uppercase BUG_* names are modernish shell bug IDs. +2021-12-27: + +- Fixed a crash or freeze that would occur on Linux when using Ctrl+C to + interrupt a command substitution containing a pipe in an interactive shell. + 2021-12-26: - Listing aliases or tracked aliases in a script no longer corrupts diff --git a/src/cmd/ksh93/include/version.h b/src/cmd/ksh93/include/version.h index c6a91870b..e392006eb 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 "2021-12-26" /* must be in this format for $((.sh.version)) */ +#define SH_RELEASE_DATE "2021-12-27" /* must be in this format for $((.sh.version)) */ #define SH_RELEASE_CPYR "(c) 2020-2021 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/subshell.c b/src/cmd/ksh93/sh/subshell.c index fa005322f..669835690 100644 --- a/src/cmd/ksh93/sh/subshell.c +++ b/src/cmd/ksh93/sh/subshell.c @@ -492,7 +492,7 @@ Sfio_t *sh_subshell(Shell_t *shp,Shnode_t *t, volatile int flags, int comsub) int *saveexitval = job.exitval; char **savsig; Sfio_t *iop=0; - struct checkpt buff; + struct checkpt checkpoint; struct sh_scoped savst; struct dolnod *argsav=0; int argcnt; @@ -509,7 +509,7 @@ Sfio_t *sh_subshell(Shell_t *shp,Shnode_t *t, volatile int flags, int comsub) } shp->curenv = ++subenv; savst = shp->st; - sh_pushcontext(shp,&buff,SH_JMPSUB); + sh_pushcontext(&sh,&checkpoint,SH_JMPSUB); shp->subshell++; /* increase level of virtual subshells */ shgd->realsubshell++; /* increase ${.sh.subshell} */ sp->prev = subshell_data; @@ -605,7 +605,7 @@ Sfio_t *sh_subshell(Shell_t *shp,Shnode_t *t, volatile int flags, int comsub) shp->cpid = 0; sh_sigreset(0); } - jmpval = sigsetjmp(buff.buff,0); + jmpval = sigsetjmp(checkpoint.buff,0); if(jmpval==0) { if(comsub) @@ -681,10 +681,10 @@ Sfio_t *sh_subshell(Shell_t *shp,Shnode_t *t, volatile int flags, int comsub) sh_trap(trap,0); free(trap); } - sh_popcontext(shp,&buff); - if(shp->subshell==0) /* must be child process */ + if(sh.subshell==0) /* we must have forked with sh_subfork(); this is the child process */ { subshell_data = sp->prev; + sh_popcontext(&sh,&checkpoint); if(jmpval==SH_JMPSCRIPT) siglongjmp(*shp->jmplist,jmpval); shp->exitval &= SH_EXITMASK; @@ -886,16 +886,14 @@ Sfio_t *sh_subshell(Shell_t *shp,Shnode_t *t, volatile int flags, int comsub) } shp->subshare = sp->subshare; shp->subdup = sp->subdup; - if(shp->subshell) - { - shp->subshell--; /* decrease level of virtual subshells */ - shgd->realsubshell--; /* decrease ${.sh.subshell} */ - } + sh.subshell--; /* decrease level of virtual subshells */ + shgd->realsubshell--; /* decrease ${.sh.subshell} */ subshell_data = sp->prev; + sh_popcontext(&sh,&checkpoint); if(!argsav || argsav->dolrefcnt==argcnt) sh_argfree(shp,argsav,0); - if(shp->topfd != buff.topfd) - sh_iorestore(shp,buff.topfd|IOSUBSHELL,jmpval); + if(sh.topfd != checkpoint.topfd) + sh_iorestore(&sh,checkpoint.topfd|IOSUBSHELL,jmpval); if(sp->sig) { if(sp->prev)