From b09ce2fa022133df4882598b9eb177e3a673e9b3 Mon Sep 17 00:00:00 2001 From: Martijn Dekker Date: Thu, 17 Feb 2022 20:07:56 +0000 Subject: [PATCH] Fix crash when suspending a blocked write to a FIFO Reproducer (symptoms on at least macOS and FreeBSD): $ mkfifo f $ echo foo > f (press Ctrl+Z) ^Zksh: f: cannot create [Interrupted system call] Abort The shell either aborts (dev builds) or crashes with 'Illegal instruction' (release builds). This is consistent with UNREACHABLE() being reached. Backtrace: 0 libsystem_kernel.dylib __kill + 10 1 ksh sh_done + 836 (fault.c:678) 2 ksh sh_fault + 1324 3 libsystem_platform.dylib _sigtramp + 29 4 dyld ImageLoaderMachOCompressed::resolve(ImageLoader::LinkContext const&, char const*, unsigned ch 5 libsystem_c.dylib abort + 127 6 ksh sh_redirect + 3576 (io.c:1356) 7 ksh sh_exec + 7231 (xec.c:1308) 8 ksh exfile + 3247 (main.c:607) 9 ksh sh_main + 3551 (main.c:368) 10 ksh main + 38 (pmain.c:45) 11 libdyld.dylib start + 1 This means that UNREACHABLE() is actually reached here: ksh/src/cmd/ksh93/sh/io.c 1351: if((fd=sh_open(tname?tname:fname,o_mode,RW_ALL)) <0) 1352: { 1353: errormsg(SH_DICT,ERROR_system(1),((o_mode&O_CREAT)?e_create:e_open),fname); 1354: UNREACHABLE(); 1355: } The cause is that, in the following section of code in sh_fault(): ksh/src/cmd/ksh93/sh/fault.c 183: #ifdef SIGTSTP 184: if(sig==SIGTSTP) 185: { 186: sh.trapnote |= SH_SIGTSTP; 187: if(pp->mode==SH_JMPCMD && sh_isstate(SH_STOPOK)) 188: { 189: sigrelease(sig); 190: sh_exit(SH_EXITSIG); 191: return; 192: } 193: } 194: #endif /* SIGTSTP */ ...sh_exit() is not getting called and the function will not return because the SH_STOPOK bit is not set while the shell is blocked waiting to write to a FIFO. Even if sh_exit() did get called, that would not fix it, because that function also checks for the SH_STOPOK bit and returns without doing a longjmp if the signal is SIGTSTP and the SH_STOPOK bit is not set. That is direct the reason why UNREACHABLE() was raeched: errormsg() does call sh_exit() but sh_exit() then does not longjmp. src/cmd/ksh93/sh/fault.c: sh_fault(): - To avoid the crash, we simply need to return from sh_fault() if SH_STOPOK is off, so that the code path does not continue, no error message is given on Ctrl+Z, UNREACHABLE() is not reached, and the shell resumes waiting on trying to write to the FIFO. The sh.trapnote flag should not be set if we're not going to process the signal. This makes ksh behave like all other shells. Resolves: https://github.com/ksh93/ksh/issues/464 --- NEWS | 3 +++ src/cmd/ksh93/sh/fault.c | 8 ++++---- src/cmd/ksh93/tests/pty.sh | 21 +++++++++++++++++++++ 3 files changed, 28 insertions(+), 4 deletions(-) diff --git a/NEWS b/NEWS index d0fbee9e3..5c28ac7ae 100644 --- a/NEWS +++ b/NEWS @@ -8,6 +8,9 @@ Any uppercase BUG_* names are modernish shell bug IDs. - Fixed a crash, introduced on 2021-01-19, that occurred when using 'cd' in a subshell with the PWD variable unset. +- Fixed a crash that could occur when or after entering the suspend character + (Ctrl+Z) while the shell was blocked trying to write to a FIFO special file. + 2022-01-16: - Backported minor additions to the 'read' built-in command from ksh 93v-: diff --git a/src/cmd/ksh93/sh/fault.c b/src/cmd/ksh93/sh/fault.c index f5c4ee4c2..1ae545160 100644 --- a/src/cmd/ksh93/sh/fault.c +++ b/src/cmd/ksh93/sh/fault.c @@ -181,15 +181,15 @@ void sh_fault(register int sig) sh.lastsig = sig; flag = SH_SIGSET; #ifdef SIGTSTP - if(sig==SIGTSTP) + if(sig==SIGTSTP && pp->mode==SH_JMPCMD) { - sh.trapnote |= SH_SIGTSTP; - if(pp->mode==SH_JMPCMD && sh_isstate(SH_STOPOK)) + if(sh_isstate(SH_STOPOK)) { + sh.trapnote |= SH_SIGTSTP; sigrelease(sig); sh_exit(SH_EXITSIG); - return; } + return; } #endif /* SIGTSTP */ } diff --git a/src/cmd/ksh93/tests/pty.sh b/src/cmd/ksh93/tests/pty.sh index 06f316bb1..aed6a2662 100755 --- a/src/cmd/ksh93/tests/pty.sh +++ b/src/cmd/ksh93/tests/pty.sh @@ -977,5 +977,26 @@ r !99 r : !99: event not found\r\n$ ! +mkfifo testfifo +tst $LINENO <<"!" +L suspend a blocked write to a FIFO +# https://github.com/ksh93/ksh/issues/464 + +d 15 +p :test-1: +w echo >testfifo +r echo +# untrapped SIGTSTP (Ctrl+Z) should be ineffective here and just print ^Z +c \cZ +r ^\^Z$ +# Ctrl+C should interrupt it and trigger an error message +c \cC +r ^\^C.*: testfifo: cannot create \[.*\]\r\n$ +p :test-2: +w echo ok +r echo +r ^ok\r\n$ +! + # ====== exit $((Errors<125?Errors:125))