From 225323f13836e3ccd77ec8c32520819d9ec08793 Mon Sep 17 00:00:00 2001 From: Martijn Dekker Date: Mon, 20 Jun 2022 18:41:28 +0100 Subject: [PATCH] Fix more "/dev/null: cannot create" (re: 411481eb) Reproducer: trap : USR1 while :; do kill -s USR1 $$ || exit; done & while :; do : >/dev/null; done It can take between a fraction of a second and a few minutes, but eventually it will fail like this: $ ksh foo foo[3]: /dev/null: cannot create kill: 77946: no such process It fails similarly with "cannot open" if /dev/null. This is the same problem as in the referenced commit, except when handling traps -- so the same fix is required in sh_fault(). --- NEWS | 5 +++++ src/cmd/ksh93/include/version.h | 2 +- src/cmd/ksh93/sh/fault.c | 33 ++++++++++++++++++++++----------- 3 files changed, 28 insertions(+), 12 deletions(-) diff --git a/NEWS b/NEWS index 3a0255692..b21cc6b59 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-06-20: + +- Fixed a race condition that could cause redirections to fail with a + "cannot create" or "cannot open" error while processing a signal trap. + 2022-06-18: - Fixed a bug where, with the monitor or pipefail option on, the shell diff --git a/src/cmd/ksh93/include/version.h b/src/cmd/ksh93/include/version.h index 81eebcff8..84543317b 100644 --- a/src/cmd/ksh93/include/version.h +++ b/src/cmd/ksh93/include/version.h @@ -23,7 +23,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-06-18" /* must be in this format for $((.sh.version)) */ +#define SH_RELEASE_DATE "2022-06-20" /* 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/fault.c b/src/cmd/ksh93/sh/fault.c index 27c03f734..6e0ce38c8 100644 --- a/src/cmd/ksh93/sh/fault.c +++ b/src/cmd/ksh93/sh/fault.c @@ -67,6 +67,7 @@ void sh_fault(register int sig) register char *trap; register struct checkpt *pp = (struct checkpt*)sh.jmplist; int action=0; + int save_errno = errno; /* reset handler */ if(!(sig&SH_TRAP)) signal(sig, sh_fault); @@ -90,7 +91,7 @@ void sh_fault(register int sig) /* critical region, save and process later */ if(!(sh.sigflag[sig]&SH_SIGIGNORE)) sh.savesig = sig; - return; + goto done; } if(sig==SIGALRM && sh.bltinfun==b_sleep) { @@ -99,18 +100,18 @@ void sh_fault(register int sig) sh.trapnote |= SH_SIGTRAP; sh.sigflag[sig] |= SH_SIGTRAP; } - return; + goto done; } if(sh.subshell && trap && sig!=SIGINT && sig!=SIGQUIT && sig!=SIGWINCH && sig!=SIGCONT) { sh.exitval = SH_EXITSIG|sig; sh_subfork(); sh.exitval = 0; - return; + goto done; } /* handle ignored signals */ if(trap && *trap==0) - return; + goto done; flag = sh.sigflag[sig]&~SH_SIGOFF; if(!trap) { @@ -119,7 +120,7 @@ void sh_fault(register int sig) if(sh.subshell) sh.ignsig = sig; sigrelease(sig); - return; + goto done; } if(flag&SH_SIGDONE) { @@ -129,7 +130,7 @@ void sh_fault(register int sig) /* check for TERM signal between fork/exec */ if(sig==SIGTERM && job.in_critical) sh.trapnote |= SH_SIGTERM; - return; + goto done; } sh.lastsig = sig; sigrelease(sig); @@ -162,7 +163,7 @@ void sh_fault(register int sig) dp->exceptf = malloc_done; } #endif - return; + goto done; } } errno = 0; @@ -190,7 +191,7 @@ void sh_fault(register int sig) sigrelease(sig); sh_exit(SH_EXITSIG); } - return; + goto done; } #endif /* SIGTSTP */ } @@ -198,12 +199,12 @@ void sh_fault(register int sig) if((error_info.flags&ERROR_NOTIFY) && sh.bltinfun) action = (*sh.bltinfun)(-sig,(char**)0,(void*)0); if(action>0) - return; + goto done; #endif if(sh.bltinfun && sh.bltindata.notify) { sh.bltindata.sigset = 1; - return; + goto done; } sh.trapnote |= flag; if(sig <= sh.sigmax) @@ -211,10 +212,20 @@ void sh_fault(register int sig) if(pp->mode==SH_JMPCMD && sh_isstate(SH_STOPOK)) { if(action<0) - return; + goto done; sigrelease(sig); sh_exit(SH_EXITSIG); } +done: + /* + * Always restore errno, because this code is run during signal handling which may interrupt loops like: + * while((fd = open(path, flags, mode)) < 0) + * if(errno!=EINTR) + * ; + * otherwise that may fail if a signal is caught between the open() call and the errno!=EINTR check. + */ + errno = save_errno; + return; } /*