1
0
Fork 0
mirror of git://git.code.sf.net/p/cdesktopenv/code synced 2025-02-15 04:32:24 +00:00

Fix race conditions running external commands with job control on

When ksh is compiled with SHOPT_SPAWN (the default), which uses
posix_spawn(3) or vfork(2) (via sh_ntfork()) to launch external
commands, at least two race conditions occur when launching
external commands while job control is active. See:
https://bugs.launchpad.net/ubuntu/+source/ksh/+bug/1887863/comments/3
https://www.mail-archive.com/ast-developers@research.att.com/msg00717.html

The basic issue is that this performance optimisation is
incompatible with job control, because it uses a spawning mechanism
that doesn't copy the parent process' memory pages into the child
process, therefore no state that involves memory can be set before
exec-ing the external program. This makes it impossible to
correctly set the terminal's process group ID in the child process,
something that is essential for job control to work.

src/cmd/ksh93/sh/xec.c:
- Use sh_fork() instead of sh_ntfork() if job control is active.
  This uses fork(2), which is 30%-ish slower on most sytems, but
  allows for correctly setting the terminal process group.

src/cmd/ksh93/tests/basic.sh:
- Add regression test for the race condition reported in #79.

src/cmd/INIT/cc.darwin:
- Remove hardcoded flag to disable SHOPT_SPAWN on the Mac.
  It should be safe to use now.

Fixes https://github.com/ksh93/ksh/issues/79
This commit is contained in:
Martijn Dekker 2020-07-22 12:47:04 +01:00
parent 4e5f24e38c
commit f207cd5787
5 changed files with 26 additions and 3 deletions

5
NEWS
View file

@ -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.
2020-07-22:
- Fixed two race conditions when running external commands on
interactive shells with job control active.
2020-07-20:
- If a shell function and a built-in command by the same name exist,

View file

@ -43,7 +43,7 @@ init) echo "cc: arguments expected" >&2
;;
cpp) $CC -E "$@"
;;
cc) $CC -DSHOPT_SPAWN=0 -D_ast_int8_t=int64_t -D_lib_memccpy \
cc) $CC -D_ast_int8_t=int64_t -D_lib_memccpy \
-Wno-unused-value -Wno-parentheses -Wno-macro-redefined "$@"
;;
dll) $CC -Wl,-flat_namespace -dynamiclib -undefined dynamic_lookup "$@"

View file

@ -17,4 +17,4 @@
* David Korn <dgk@research.att.com> *
* *
***********************************************************************/
#define SH_RELEASE "93u+m 2020-07-20"
#define SH_RELEASE "93u+m 2020-07-22"

View file

@ -1575,7 +1575,7 @@ int sh_exec(register const Shnode_t *t, int flags)
}
#if SHOPT_SPAWN
# ifdef _lib_fork
if(com)
if(com && !job.jobcontrol)
parent = sh_ntfork(shp,t,com,&jobid,ntflag);
else
parent = sh_fork(shp,type,&jobid);
@ -3460,6 +3460,7 @@ static void sigreset(Shell_t *shp,int mode)
/*
* A combined fork/exec for systems with slow or non-existent fork()
* Note: Incompatible with job control.
*/
static pid_t sh_ntfork(Shell_t *shp,const Shnode_t *t,char *argv[],int *jobid,int flag)
{

View file

@ -598,5 +598,22 @@ eu=$(
)
[[ ${us:1:1} == ${eu:1:1} ]] && err_exit "The time keyword ignores the locale's radix point (both are ${eu:1:1})"
# ======
# Test for bug in ksh binaries that use posix_spawn() while job control is active.
# See discussion at: https://github.com/ksh93/ksh/issues/79
if test -t 1 2>/dev/null 1>/dev/tty # this test only works if we have a tty
then actual=$(
"$SHELL" -i <<-\EOF 2>/dev/tty
printf '%s\n' 1 2 3 4 5 | while read
do ls /dev/null
done 2>&1
exit # suppress extra newline
EOF
)
expect=$'/dev/null\n/dev/null\n/dev/null\n/dev/null\n/dev/null'
[[ $actual == "$expect" ]] || err_exit 'Race condition while launching external commands' \
"(expected $(printf %q "$expect"), got $(printf %q "$actual"))"
fi
# ======
exit $((Errors<125?Errors:125))