From d024d4c895ca78df173d0ee71fa95fbe8089944d Mon Sep 17 00:00:00 2001 From: Martijn Dekker Date: Mon, 8 Jun 2020 12:23:37 +0200 Subject: [PATCH] Fix signal handling due to exit status > 256 This fixes two bugs: issuing the 'exit' command with a value > 256 would cause ksh 93u+ to kill itself with the corresponding signal (try 'exit 265' to SIGKILL your interactive shell), and, if the last command of a script exits due to a signal, the shell would repeat that signal to itself, causing any parent ksh to also be killed. Discussion: https://bugzilla.redhat.com/show_bug.cgi?id=1469624 https://rainbow.chard.org/2017/03/21/ksh-deliberately-segfaults-if-the-last-command-in-a-script-crashes/ This commit is loosely based on a patch applied to the 93v- beta and the abandoned ksh2020, but that patch was incomplete & broken: $ ksh-2020.0.0 -c 'exit 265'; echo $? 137 Expected: 9. Since the exit was *not* due to a signal, the value should simply be cropped to the 8 bits supported by the OS. src/cmd/ksh93/bltins/cflow.c: b_exit(): - For the 'exit' builtin command, bitwise-AND the argument to 'exit' with SH_EXITMASK (8 bits, crop to 0-255) before passing it on to sh_exit(). This restores the behaviour of <=2011 ksh93 versions and is in line with all other POSIX shells. It also fixes this bogosity: $ (exit 265); echo $? # non-forked subshell 265 $ (ulimit -t unlimited; exit 265); echo $? # forked subshell 9 Forked or non-forked should make no difference at all (see commit message a0e0e29e for why). src/cmd/ksh93/sh/fault.c: sh_done(): - If the current exit status is equal to the status for the last signal that was received from a child process, remove the SH_EXITSIG (9th) bit, so that the shell doesn't kill itself. - If the shell's last child process exits due to a signal, exit with a portable 8-bit exit status (128 + signal number). This avoids the exit status being < 128 by being cropped to 8 bits. src/cmd/ksh93/tests/signal.sh: - Add regression test for exit with status > 256. - Add regression test verifying the shell no longer kills itself. (cherry picked from commit 98e0fc94393e175ce6adfee390327c320795bf12) --- NEWS | 4 ++++ src/cmd/ksh93/bltins/cflow.c | 2 +- src/cmd/ksh93/sh/fault.c | 7 ++++++- src/cmd/ksh93/tests/signal.sh | 22 ++++++++++++++++++++++ 4 files changed, 33 insertions(+), 2 deletions(-) diff --git a/NEWS b/NEWS index 7fb4f5573..6d4aa366f 100644 --- a/NEWS +++ b/NEWS @@ -20,6 +20,10 @@ Any uppercase BUG_* names are modernish shell bug IDs. - undocumented builtins 'vmap' and 'vpath' that only printed error messages - a non-functional -V unary operator for the test and [[ commands +- If the last program run by a ksh script exits with a signal (e.g. crashed), + ksh itself now exits normally instead of repeating that same signal. + In addition, using 'exit x' for x > 256 no longer makes ksh issue a signal. + 2020-06-06: - The 'times' command is now a builtin command that conforms to POSIX diff --git a/src/cmd/ksh93/bltins/cflow.c b/src/cmd/ksh93/bltins/cflow.c index d76f98532..fb2fccc6b 100644 --- a/src/cmd/ksh93/bltins/cflow.c +++ b/src/cmd/ksh93/bltins/cflow.c @@ -70,7 +70,7 @@ done: /* return outside of function, dotscript and profile is exit */ if(shp->fn_depth==0 && shp->dot_depth==0 && !sh_isstate(SH_PROFILE)) pp->mode = SH_JMPEXIT; - sh_exit(shp->savexit=n); + sh_exit((shp->savexit = n) & SH_EXITMASK); return(1); } diff --git a/src/cmd/ksh93/sh/fault.c b/src/cmd/ksh93/sh/fault.c index 8e8ee510c..30c536a7b 100644 --- a/src/cmd/ksh93/sh/fault.c +++ b/src/cmd/ksh93/sh/fault.c @@ -643,7 +643,7 @@ void sh_done(void *ptr, register int sig) sfsync((Sfio_t*)sfstdin); sfsync((Sfio_t*)shp->outpool); sfsync((Sfio_t*)sfstdout); - if(savxit&SH_EXITSIG) + if(savxit&SH_EXITSIG && (savxit&SH_EXITMASK) == shp->lastsig) sig = savxit&SH_EXITMASK; if(sig) { @@ -668,6 +668,11 @@ void sh_done(void *ptr, register int sig) if(sh_isoption(SH_NOEXEC)) kiaclose((Lex_t*)shp->lex_context); #endif /* SHOPT_KIA */ + + /* Exit with portable 8-bit status (128 + signum) if last child process exits due to signal */ + if (savxit & SH_EXITSIG) + savxit -= SH_EXITSIG + 128; + exit(savxit&SH_EXITMASK); } diff --git a/src/cmd/ksh93/tests/signal.sh b/src/cmd/ksh93/tests/signal.sh index 87cfd7d92..1e35b212c 100755 --- a/src/cmd/ksh93/tests/signal.sh +++ b/src/cmd/ksh93/tests/signal.sh @@ -437,4 +437,26 @@ a [[ $endb ]] && err_exit 'TERM signal did not kill function b' [[ $enda == 1 ]] || err_exit 'TERM signal killed function a' +# ====== +# Exit status checks + +# Verify that 'exit x' for x > 256 does not make the shell send a signal to itself +"$SHELL" -c 'exit $((256+9))' +let "$? == 256+9" && err_exit 'exit with status > 256 makes shell kill itself' + +# Verify that the shell does not kill itself after detecting that a child process is killed by a signal, +# and that a signal still causes the exit status to be set to a value > 128 +cat >"$tmp/sigtest.sh" <<\EOF +echo begin +sh -c 'kill -9 "$$"' +EOF +expect=$'^begin\n/.*/sigtest.sh: line 2: [1-9][0-9]*: Killed\n[1-9][0-9]{1,2}$' +actual=$("$SHELL" -c 'ksh "$1"; echo "$?"' x "$tmp/sigtest.sh" 2>&1) +if ! [[ $actual =~ $expect ]] +then [[ $actual == *Killed*Killed* ]] && msg='ksh killed itself' || msg='unexpected output' + err_exit "$msg after child process signal (expected match to $(printf %q "$expect"); got $(printf %q "$actual"))" +fi +let "${actual##*$'\n'} > 128" || err_exit "child process signal did not cause exit status > 128" + +# ====== exit $((Errors<125?Errors:125))