From 896e6a2c0867cdce50520315605adada759132a2 Mon Sep 17 00:00:00 2001 From: Johnothan King Date: Mon, 16 Jun 2025 15:35:25 -0700 Subject: [PATCH 1/5] Overhaul spawnveg to use clone(2) directly; fix many related spawning bugs Alterations: - Add a version of spawnveg that clones processes with clone(2) outright. This method is generally faster than using posix_spawn, and is also more portable (since Linux with musl libc *and* NetBSD both provide this function). The code was initially based on the _real_vfork spawnveg code previously removed in ef9a5c76 (unlike vfork(2), there is no problem with running code prior to execve for clone with CLONE_VM|CLONE_VFORK). - Remove the now dead code implementing tcsetpgrp via posix_spawn_file_actions_addtcsetpgrp_np(). This code was only ever used with glibc 2.35+. (Not only that, but the only other platform that ever considered implementing such a call was NetBSD[^1], where it appears to have been abandoned. This is ironic considering ksh no longer uses posix_spawn on NetBSD as of this commit.) This code may be worth visiting if io_uring_spawn()[^2] is ever actualized (that project appears to be quite inactive though). - Bugfix: Don't set the terminal signals to their defaults in the parent prior to calling spawnveg. This is the primary cause of a lockup in the pty tests which can be reproduced as follows: $ exec ksh $ bin/shtests pty 2>/dev/null ^C ^C ^C (SIGINT doesn't work anymore and may segfault) The correct way to go about dealing with SIGT* is to set those to SIG_DFL in the child process (cf. _sh_fork()). - Added support for tcsetpgrp to the fork fallback in spawnveg. Some form of this appears to have already been attempted in AT&T olden times, but that old version was broken and needed bugfixes desperately. - If the child needs tcsetpgrp, block the terminal signals in the parent process via sigcritical(). The posix_spawn version doesn't need this because posix_spawn will block signals automatically and therefore doesn't need sigcritical. - Now that the fork fallback for spawnveg works correctly in interactive terminals, prefer that to the sh_fork() codepath on operating systems without posix_spawn tcsetpgrp support. Even though the underlying system call is still ultimately fork, the sh_ntfork() codepath is faster than the traditional sh_fork codepath. Benchmark: $ time /tmp/ksh-devbranch -ic "for i in {1..10000}; do $(whence -p true); done" real 0m03.302s user 0m00.988s sys 0m02.320s $ time /tmp/ksh-newspawnveg -ic "for i in {1..10000}; do $(whence -p true); done" real 0m03.160s user 0m01.187s sys 0m01.977s - To that end, split up the spawnveg versions into spawnveg_fast and spawnveg_slow. Choose the appropriate one when spawnveg is called; this removes the need for the xec.c ifdef hackery. - Removed the ntfork_tcpgrp ifdefs from xec.c; spawnveg can handle it by itself now. - Bugfix: With the spawnveg_fast and spawnveg_slow innovation, spawnveg now always has support for setsid. It'll fallback to fork if POSIX_SPAWN_SETSID and clone(2) aren't available. - Bugfix: For the posix_spawn version of spawnveg, the flags should be of the short type pursuant to the POSIX specification. - Optimization: Use pipe2 in the fork fallback for spawnveg when it's available to avoid two fcntl syscalls. - Updated the spawnveg documentation to reflect the new changes. - _spawnveg(): Restore the terminal process group immediately after any failed spawn attempt, rather than only in sh_ntfork(). - Added a regression test for a crash that occurred because of the erroneous tcsetpgrp placement in sh_ntfork(). - path_spawn(): Do not print an error message and longjmp away upon encountering E2BIG or some other error; let the calling functions take care of that. - path_exec(): Added handling for E2BIG. - Fixed a bug that caused sh_ntfork to leak file descriptors upon encountering an error (re: 8f848bc). Reproducer: #!/bin/ksh ls /proc/$$/fd redirect 2>/dev/null touch /tmp/file for i in {1..20}; do command /tmp/file <(echo) done ls /proc/$$/fd - Added regression tests for the leak that use 'command' and 'command -x'. - Fixed an old regression test that was giving false negatives. - Clarified the intent of the sigcrit nesting matches. The ksh2020 devs appeared to have been confused by this line, so some additional clarification explaining the intent should be helpful for posterity. - path.sh: Added tests for the exit status of commands run when forked with the & operator. [^1]: https://blog.netbsd.org/tnf/entry/gsoc_reports_make_system_31 [^2]: https://lwn.net/Articles/908268/ --- NEWS | 12 ++ src/cmd/ksh93/include/version.h | 2 +- src/cmd/ksh93/sh/path.c | 36 ++-- src/cmd/ksh93/sh/xec.c | 79 ++------- src/cmd/ksh93/tests/io.sh | 61 ++++++- src/cmd/ksh93/tests/path.sh | 35 +++- src/cmd/ksh93/tests/pty.sh | 13 ++ src/lib/libast/Mamfile | 1 + src/lib/libast/comp/spawnveg.c | 286 ++++++++++++++++++++++---------- src/lib/libast/features/lib | 5 +- src/lib/libast/man/spawnveg.3 | 47 +++--- src/lib/libast/misc/sigcrit.c | 9 +- 12 files changed, 384 insertions(+), 202 deletions(-) diff --git a/NEWS b/NEWS index 81c063c046d0..b6d7945c466d 100644 --- a/NEWS +++ b/NEWS @@ -2,6 +2,18 @@ This documents significant changes in the dev branch of ksh 93u+m. For full details, see the git log at: https://github.com/ksh93/ksh Uppercase BUG_* IDs are shell bug IDs as used by the Modernish shell library. +2025-07-03: + +- Fixed a regression introduced on 02-02-2022 occurring on systems with + glibc 2.35+ that could cause the pty regression tests to crash or lockup. + +- Fixed a regression introduced on 02-02-2022 that could cause ksh to crash + after a command fails with E2BIG. + +- Fixed a bug that caused a file descriptor leak when a command run + with the 'command' builtin couldn't be executed due to an error + (e.g. ENOENT or E2BIG). + 2025-06-14: - Fixed a bug occurring on systems with posix_spawn(3), spawnve(2), and diff --git a/src/cmd/ksh93/include/version.h b/src/cmd/ksh93/include/version.h index 845e905acb8f..25a5f14e2c09 100644 --- a/src/cmd/ksh93/include/version.h +++ b/src/cmd/ksh93/include/version.h @@ -18,7 +18,7 @@ #include #include "git.h" -#define SH_RELEASE_DATE "2025-06-22" /* must be in this format for $((.sh.version)) */ +#define SH_RELEASE_DATE "2025-07-03" /* must be in this format for $((.sh.version)) */ /* * This comment keeps SH_RELEASE_DATE a few lines away from SH_RELEASE_SVER to avoid * merge conflicts when cherry-picking dev branch commits onto a release branch. diff --git a/src/cmd/ksh93/sh/path.c b/src/cmd/ksh93/sh/path.c index 298bbd2e2b60..06304db49d58 100644 --- a/src/cmd/ksh93/sh/path.c +++ b/src/cmd/ksh93/sh/path.c @@ -87,6 +87,8 @@ static pid_t _spawnveg(const char *path, char* const argv[], char* const envp[], if(pid>=0 || errno!=EAGAIN) break; } + if(pid<0 && job.jobcontrol) + tcsetpgrp(job.fd,sh.pid); /* if spawnveg set tcpgrp, we must restore it ourselves */ return pid; } @@ -993,15 +995,20 @@ noreturn void path_exec(const char *arg0,char *argv[],struct argnod *local) if(sh.subshell) sh_subtmpfile(); spawnpid = path_spawn(opath,argv,envp,libpath,0); - if(spawnpid==-1 && sh.path_err!=ENOENT) + if(spawnpid==-1) { - /* - * A command was found but it couldn't be executed. - * POSIX specifies that the shell should continue to search for the - * command in PATH and return 126 only when it can't find an executable - * file in other elements of PATH. - */ - not_executable = sh.path_err; + if(sh.path_err == E2BIG) + break; + else if(sh.path_err!=ENOENT) + { + /* + * A command was found but it couldn't be executed. + * POSIX specifies that the shell should continue to search for the + * command in PATH and return 126 only when it can't find an executable + * file in other elements of PATH. + */ + not_executable = sh.path_err; + } } while(pp && (pp->flags&PATH_FPATH)) pp = path_nextcomp(pp,arg0,0); @@ -1258,13 +1265,6 @@ pid_t path_spawn(const char *opath,char **argv, char **envp, Pathcomp_t *libpath case EPERM: sh.path_err = errno; return -1; - case ENOTDIR: - case ENOENT: - case EINTR: -#ifdef EMLINK - case EMLINK: -#endif /* EMLINK */ - return -1; case E2BIG: if(sh_isstate(SH_XARG)) { @@ -1282,10 +1282,10 @@ pid_t path_spawn(const char *opath,char **argv, char **envp, Pathcomp_t *libpath } /* FALLTHROUGH */ default: - errormsg(SH_DICT,ERROR_system(ERROR_NOEXEC),e_exec,path); - UNREACHABLE(); + sh.path_err = errno; + return -1; } - return 0; + UNREACHABLE(); } /* diff --git a/src/cmd/ksh93/sh/xec.c b/src/cmd/ksh93/sh/xec.c index 1f78e3a738f4..c3ac9a7f6cd5 100644 --- a/src/cmd/ksh93/sh/xec.c +++ b/src/cmd/ksh93/sh/xec.c @@ -44,11 +44,6 @@ # include #endif -#undef _use_ntfork_tcpgrp -#if SHOPT_SPAWN && _lib_posix_spawn > 1 && _lib_posix_spawn_file_actions_addtcsetpgrp_np -#define _use_ntfork_tcpgrp 1 -#endif - #if SHOPT_SPAWN static pid_t sh_ntfork(const Shnode_t*,char*[],int*,int); #endif /* SHOPT_SPAWN */ @@ -1437,11 +1432,7 @@ int sh_exec(const Shnode_t *t, int flags) fifo_save_ppid = sh.current_pid; #endif #if SHOPT_SPAWN -#if _use_ntfork_tcpgrp if(com) -#else - if(com && !job.jobcontrol) -#endif /* _use_ntfork_tcpgrp */ { parent = sh_ntfork(t,com,&jobid,topfd); if(parent<0) @@ -3273,9 +3264,12 @@ static void sigreset(int mode) } /* - * A combined fork/exec for systems with slow fork(). - * Incompatible with job control on interactive shells (job.jobcontrol) if - * the system does not support posix_spawn_file_actions_addtcsetpgrp_np(). + * A combined fork/exec which utilizes libast spawnveg(3) for better performance. + * The spawnveg function will invoke posix_spawn(3) or an equivalent if possible, + * and will fallback to fork(2) if absolutely necessary. For simple command + * execution this codepath is prefered because it's always a bit faster than + * the sh_fork() codepath, even when the underlying system calls it uses wind up + * being the same. */ static pid_t sh_ntfork(const Shnode_t *t,char *argv[],int *jobid,int topfd) { @@ -3286,9 +3280,6 @@ static pid_t sh_ntfork(const Shnode_t *t,char *argv[],int *jobid,int topfd) char **arge, *path; volatile pid_t grp = 0; Pathcomp_t *pp; -#if _use_ntfork_tcpgrp - volatile int jobwasset=0; -#endif /* _use_ntfork_tcpgrp */ sh_pushcontext(buffp,SH_JMPCMD); errorpush(&buffp->err,ERROR_SILENT); job_lock(); /* errormsg will unlock */ @@ -3340,14 +3331,6 @@ static pid_t sh_ntfork(const Shnode_t *t,char *argv[],int *jobid,int topfd) } arge = sh_envgen(); sh.exitval = 0; -#if _use_ntfork_tcpgrp - if(job.jobcontrol) - { - signal(SIGTTIN,SIG_DFL); - signal(SIGTTOU,SIG_DFL); - signal(SIGTSTP,SIG_DFL); - jobwasset++; - } if(sh_isstate(SH_MONITOR) && job.jobcontrol) { if(job.curpgid==0) @@ -3355,7 +3338,6 @@ static pid_t sh_ntfork(const Shnode_t *t,char *argv[],int *jobid,int topfd) else grp = job.curpgid; } -#endif /* _use_ntfork_tcpgrp */ sfsync(NULL); sigreset(0); /* set signals to ignore */ @@ -3368,35 +3350,19 @@ static pid_t sh_ntfork(const Shnode_t *t,char *argv[],int *jobid,int topfd) fail: if(jobfork && spawnpid<0) job_fork(-2); - if(spawnpid == -1) + if(spawnpid==-1) switch(errno=sh.path_err) { -#if _use_ntfork_tcpgrp - if(jobwasset) - { - signal(SIGTTIN,SIG_IGN); - signal(SIGTTOU,SIG_IGN); - if(sh_isstate(SH_INTERACTIVE)) - signal(SIGTSTP,SIG_IGN); - else - signal(SIGTSTP,SIG_DFL); - } - if(job.jobcontrol) - tcsetpgrp(job.fd,sh.pid); -#endif /* _use_ntfork_tcpgrp */ - switch(errno=sh.path_err) - { - case ENOENT: - errormsg(SH_DICT,ERROR_exit(ERROR_NOENT),e_found+4); - UNREACHABLE(); + case ENOENT: + errormsg(SH_DICT,ERROR_exit(ERROR_NOENT),e_found+4); + UNREACHABLE(); #ifdef ENAMETOOLONG - case ENAMETOOLONG: - errormsg(SH_DICT,ERROR_exit(ERROR_NOENT),e_toolong+4); - UNREACHABLE(); + case ENAMETOOLONG: + errormsg(SH_DICT,ERROR_exit(ERROR_NOENT),e_toolong+4); + UNREACHABLE(); #endif - default: - errormsg(SH_DICT,ERROR_system(ERROR_NOEXEC),e_exec+4); - UNREACHABLE(); - } + default: + errormsg(SH_DICT,ERROR_system(ERROR_NOEXEC),e_exec+4); + UNREACHABLE(); } job_unlock(); } @@ -3405,17 +3371,6 @@ static pid_t sh_ntfork(const Shnode_t *t,char *argv[],int *jobid,int topfd) sh_popcontext(buffp); if(buffp->olist) free_list(buffp->olist); -#if _use_ntfork_tcpgrp - if(jobwasset) - { - signal(SIGTTIN,SIG_IGN); - signal(SIGTTOU,SIG_IGN); - if(sh_isstate(SH_INTERACTIVE)) - signal(SIGTSTP,SIG_IGN); - else - signal(SIGTSTP,SIG_DFL); - } -#endif /* _use_ntfork_tcpgrp */ if(sigwasset) sigreset(1); /* restore ignored signals */ if(scope) @@ -3425,7 +3380,7 @@ static pid_t sh_ntfork(const Shnode_t *t,char *argv[],int *jobid,int topfd) if(jmpval==SH_JMPSCRIPT) nv_setlist(t->com.comset,NV_EXPORT|NV_IDENT|NV_ASSIGN,0); } - if(t->com.comio && (jmpval || spawnpid<=0) && sh.topfd > topfd) + if((t->com.comio || spawnpid<0) && jmpval && sh.topfd > topfd) sh_iorestore(topfd,jmpval); if(jmpval>SH_JMPCMD) siglongjmp(*sh.jmplist,jmpval); diff --git a/src/cmd/ksh93/tests/io.sh b/src/cmd/ksh93/tests/io.sh index 98a738ef2b24..ffd85b6f376d 100755 --- a/src/cmd/ksh93/tests/io.sh +++ b/src/cmd/ksh93/tests/io.sh @@ -696,16 +696,65 @@ err=$( "(got $(printf %q "$err"))" # File descriptor leak after 'command not found' with process substitution as argument -err=$( - ulimit -n 25 || exit 0 +exp=127 +expout=$( set +x PATH=/dev/null - for ((i=1; i<10; i++)) - do notfound <(:) >(:) 2> /dev/null + LINENO=1 + for ((i=1; i<20; i++)) + do notfound done 2>&1 exit 0 -) || err_exit "Process substitution leaks file descriptors when used as argument to nonexistent command" \ - "(got $(printf %q "$err"))" +) +gotout=$( + ulimit -n 18 || exit 0 + set +x + PATH=/dev/null + LINENO=1 + for ((i=1; i<20; i++)) + do notfound <(:) >(:) + done 2>&1 + exit +) +got=$? +((got==exp)) || err_exit "Process substitution leaks file descriptors when used as argument to nonexistent command" \ + "(expected exit status $exp, got status $got)" +[[ $expout == "$gotout" ]] || err_exit "Process substitution leaks file descriptors when used as argument to nonexistent command" \ + $'Diff follows:\n'"$(diff -u <(print -r -- "$expout") <(print -r -- "$gotout"))" + +# Same as above, but also test commands executed with command(1) +gotout=$( + ulimit -n 18 || exit 0 + set +x + PATH=/dev/null + LINENO=1 + for ((i=1; i<20; i++)) + do command notfound <(:) >(:) + done 2>&1 + exit +) +got=$? +((got==exp)) || err_exit "Process substitution leaks file descriptors when used as argument to nonexistent command run with command(1)" \ + "(expected exit status $exp, got status $got)" +[[ $expout == "$gotout" ]] || err_exit "Process substitution leaks file descriptors when used as argument to nonexistent command run with command(1)" \ + $'Diff follows:\n'"$(diff -u <(print -r -- "$expout") <(print -r -- "$gotout"))" + +# Now test with command -x +gotout=$( + ulimit -n 18 || exit 0 + set +x + PATH=/dev/null + LINENO=1 + for ((i=1; i<20; i++)) + do command -x notfound <(:) >(:) + done 2>&1 + exit +) +got=$? +((got==exp)) || err_exit "Process substitution leaks file descriptors when used as argument to nonexistent command run with 'command -x'" \ + "(expected exit status $exp, got status $got)" +[[ $expout == "$gotout" ]] || err_exit "Process substitution leaks file descriptors when used as argument to nonexistent command run with 'command -x'" \ + $'Diff follows:\n'"$(diff -u <(print -r -- "$expout") <(print -r -- "$gotout"))" got=$(command -x cat <(command -x echo foo) 2>&1) || err_exit "process substitution doesn't work with 'command'" \ "(got $(printf %q "$got"))" diff --git a/src/cmd/ksh93/tests/path.sh b/src/cmd/ksh93/tests/path.sh index c8561a8f1003..55f11933a578 100755 --- a/src/cmd/ksh93/tests/path.sh +++ b/src/cmd/ksh93/tests/path.sh @@ -751,10 +751,11 @@ PATH=$savePATH # POSIX: If a command is found but isn't executable, the exit status should be 126. # The tests are arranged as follows: # Test *A runs commands with the -c execve(2) optimization. -# Test *B runs commands with spawnveg (i.e., with posix_spawn(3) where available). -# Test *C runs commands with fork(2) in an interactive shell. +# Test *B runs commands in a non-interactive shell. +# Test *C runs commands in an interactive shell. # Test *D runs commands with 'command -x'. # Test *E runs commands with 'exec'. +# Test *F forks commands with '&'. # https://github.com/att/ast/issues/485 rm -rf noexecute print 'print cannot execute' > noexecute @@ -782,6 +783,10 @@ PATH=$PWD $SHELL -c 'exec noexecute' > /dev/null 2>&1 got=$? [[ $exp == $got ]] || err_exit "Test 1E: exit status of exec'd non-executable command wrong" \ "(expected $exp, got $got)" +PATH=$PWD $SHELL -c 'noexecute & wait $!; exit $?' > /dev/null 2>&1 +got=$? +[[ $exp == $got ]] || err_exit "Test 1F: exit status of forked job non-executable command wrong" \ + "(expected $exp, got $got)" # Add an empty directory where the command isn't found. PATH=$PWD:$PWD/emptydir $SHELL -c 'noexecute' > /dev/null 2>&1 @@ -806,6 +811,10 @@ PATH=$PWD:$PWD/emptydir $SHELL -c 'exec noexecute' > /dev/null 2>&1 got=$? [[ $exp == $got ]] || err_exit "Test 2E: exit status of exec'd non-executable command wrong" \ "(expected $exp, got $got)" +PATH=$PWD:$PWD/emptydir $SHELL -c 'noexecute & wait $!; exit $?' > /dev/null 2>&1 +got=$? +[[ $exp == $got ]] || err_exit "Test 2F: exit status of forked job non-executable command wrong" \ + "(expected $exp, got $got)" # If an executable command is found after a non-executable command, skip the non-executable one. print 'true' > cmddir/noexecute @@ -840,6 +849,10 @@ PATH=$PWD:$PWD/cmddir $SHELL -c 'exec noexecute' got=$? [[ $exp == $got ]] || err_exit "Test 3E: failed to run exec'd executable command after encountering non-executable command" \ "(expected $exp, got $got)" +PATH=$PWD:$PWD/cmddir $SHELL -c 'noexecute & wait $!; exit $?' +got=$? +[[ $exp == $got ]] || err_exit "Test 3F: failed to run forked job executable command after encountering non-executable command" \ + "(expected $exp, got $got)" # Same test as above, but with a directory of the same name in the PATH. rm "$PWD/noexecute" @@ -866,10 +879,14 @@ PATH=$PWD:$PWD/cmddir $SHELL -c 'exec noexecute' > /dev/null 2>&1 got=$? [[ $exp == $got ]] || err_exit "Test 4E: failed to run exec'd executable command after encountering directory with same name in PATH" \ "(expected $exp, got $got)" +PATH=$PWD:$PWD/cmddir $SHELL -c 'noexecute & wait $!; exit $?' > /dev/null 2>&1 +got=$? +[[ $exp == $got ]] || err_exit "Test 4F: failed to run executable command as forked job after encountering directory with same name in PATH" \ + "(expected $exp, got $got)" # Don't treat directories as commands. # https://github.com/att/ast/issues/757 mkdir cat -PATH=".:$PATH" cat < /dev/null || err_exit "Test 4F: directories should not be treated as executables" +PATH=".:$PATH" cat < /dev/null || err_exit "Test 4G: directories should not be treated as executables" # Test attempts to run directories located in the PATH. exp=126 @@ -895,6 +912,10 @@ PATH=$PWD $SHELL -c 'exec noexecute' > /dev/null 2>&1 got=$? [[ $exp == $got ]] || err_exit "Test 5E: exit status of exec'd non-executable command wrong" \ "(expected $exp, got $got)" +PATH=$PWD $SHELL -c 'noexecute & wait $!; exit $?' > /dev/null 2>&1 +got=$? +[[ $exp == $got ]] || err_exit "Test 5F: exit status of forked job non-executable command wrong" \ + "(expected $exp, got $got)" # Tests for attempting to run a non-existent command. exp=127 @@ -920,6 +941,10 @@ PATH=/dev/null $SHELL -c 'exec nonexist' > /dev/null 2>&1 got=$? [[ $exp == $got ]] || err_exit "Test 6E: exit status of exec'd non-existent command wrong" \ "(expected $exp, got $got)" +PATH=/dev/null $SHELL -c 'nonexist & wait $!; exit $?' > /dev/null 2>&1 +got=$? +[[ $exp == $got ]] || err_exit "Test 6F: exit status of forked job non-existent command wrong" \ + "(expected $exp, got $got)" # Tests for attempting to use a command name that's too long. name_max=$(builtin getconf 2>/dev/null; getconf NAME_MAX . 2>/dev/null || echo 255) @@ -947,6 +972,10 @@ PATH=$PWD $SHELL -c "exec $long_cmd" > /dev/null 2>&1 got=$? [[ $exp == $got ]] || err_exit "Test 7E: exit status or error message for exec'd command with long name wrong" \ "(expected $exp, got $got)" +PATH=$PWD $SHELL -c "$long_cmd & wait \$!; exit \$?" > /dev/null 2>&1 +got=$? +[[ $exp == $got ]] || err_exit "Test 7F: exit status or error message for forked job command with long name wrong" \ + "(expected $exp, got $got)" # ====== diff --git a/src/cmd/ksh93/tests/pty.sh b/src/cmd/ksh93/tests/pty.sh index cbda7f70c900..9f30ce484b57 100755 --- a/src/cmd/ksh93/tests/pty.sh +++ b/src/cmd/ksh93/tests/pty.sh @@ -1400,5 +1400,18 @@ I print r right ! +tst $LINENO << "!" +L crash after E2BIG due to failed tcpgrp restoration + +d 40 +P :test-.: +w integer savpid=$$ +w "$SHELL" +w $(type -p true) "$(awk -v ORS= 'BEGIN { for(i=0;i<1000000;i++) print "xx"; }')" +w ((savpid==$$)); print $? +I print +r ^1\r\n$ +! + # ====== exit $((Errors<125?Errors:125)) diff --git a/src/lib/libast/Mamfile b/src/lib/libast/Mamfile index 731c1bf3d800..a729887f4fb0 100644 --- a/src/lib/libast/Mamfile +++ b/src/lib/libast/Mamfile @@ -2404,6 +2404,7 @@ make install virtual make spawnveg.o make comp/spawnveg.c + prev ast_fcntl.h prev ast_tty.h prev sig.h prev include/wait.h diff --git a/src/lib/libast/comp/spawnveg.c b/src/lib/libast/comp/spawnveg.c index f4efe46791cf..3cd76601c2d5 100644 --- a/src/lib/libast/comp/spawnveg.c +++ b/src/lib/libast/comp/spawnveg.c @@ -2,7 +2,7 @@ * * * This software is part of the ast package * * Copyright (c) 1985-2012 AT&T Intellectual Property * -* Copyright (c) 2020-2024 Contributors to ksh 93u+m * +* Copyright (c) 2020-2025 Contributors to ksh 93u+m * * and is licensed under the * * Eclipse Public License, Version 2.0 * * * @@ -28,28 +28,174 @@ */ #include - -#if _lib_posix_spawn > 1 /* reports underlying exec() errors */ - -#include #include #include +#include +#include +#include + +/* + * Set the SID, PGID and TCPGRP in the child process + * after forking. + */ +static void setup_child(pid_t pgid, int tcfd) +{ + sigcritical(0); + if (pgid == -1) + setsid(); + else if (pgid) + { + if (pgid <= 1) + pgid = getpid(); + if (setpgid(0, pgid) < 0 && errno == EPERM) + setpgid(pgid, 0); + } + if (tcfd >= 0) + { + if(pgid == -1) + pgid = getpid(); + tcsetpgrp(tcfd, pgid); + signal(SIGTTIN,SIG_DFL); + signal(SIGTTOU,SIG_DFL); + signal(SIGTSTP,SIG_DFL); + } +} + +static void fork_cleanup(pid_t pid, pid_t pgid, int err) +{ + sigcritical(0); + if (pid != -1 && pgid > 0) + { + /* + * parent and child are in a race here + */ + + if (pgid == 1) + pgid = pid; + if (setpgid(pid, pgid) < 0 && pid != pgid && errno == EPERM) + setpgid(pid, pid); + } + errno = err; +} + + +static noreturn void exit_child(void) +{ + if(errno == ENOENT) + _exit(EXIT_NOTFOUND); +#ifdef ENAMETOOLONG + if(errno == ENAMETOOLONG) + _exit(EXIT_NOTFOUND); +#endif + _exit(EXIT_NOEXEC); +} + +#if _lib_clone +#define _fast_spawnveg 1 +#define STACK_SIZE 1024*56 +#include + +/* + * This version of spawnveg uses the Linux clone(2) syscall via the + * frontend wrapper provided by the libc. Using clone directly is + * more portable than posix_spawn_file_actions_addtcsetpgrp_np(). + * This implementation works on Linux (glibc and musl) and NetBSD. + * + * This function does a few things to attain better performance + * than the glibc and musl implementations of posix_spawn: + * - The child stack is allocated via a function local 'char stack[]' + * like in musl, which is faster than using mmap ala glibc. + * - The errno from a failed execve is merely stored in the + * args->err variable, which is accessible by both the parent + * and child thanks to CLONE_VM. This behavior matches + * glibc and 93u+'s _real_vfork spawnveg, and is faster than musl + * (which opens a pipe for interprocess communication; we don't + * need that). + * + * Additionally, unlike with posix_spawn we don't pay attention to + * error conditions from setpgid, tcsetpgrp, or setsid. For ksh93 + * it's preferable we spawn a process when possible, rather than + * abort prematurely. As of 7d2bb8fd the posix_spawn implementation + * will try again without POSIX_SPAWN_SETPGROUP (posix_spawn fails + * without spawning if any of the previous syscalls failed). In the + * clone(2) version we don't need to abort our spawn attempt if + * the process group couldn't be set. + * + * We also avoid cruft by assuming ksh93 is single-threaded. + * Implementations of posix_spawn try to be thread-safe, which we + * don't care about. + */ + +struct cargs +{ + const char *path; + char **argv; + char **envv; + volatile int err; + pid_t pgid; + int tcfd; +}; + +static noreturn int exec_process(void *data) +{ + struct cargs *args = (struct cargs*)data; + setup_child(args->pgid, args->tcfd); + execve(args->path, args->argv, args->envv); + args->err = errno; + exit_child(); +} pid_t -spawnveg(const char* path, char* const argv[], char* const envv[], pid_t pgid, int tcfd) +spawnveg_fast(const char* path, char* const argv[], char* const envv[], pid_t pgid, int tcfd) { - int err, flags = 0; + int n = errno; + pid_t pid; + char stack[STACK_SIZE]; + struct cargs args = { + .path = path, + .argv = (char**)argv, + .envv = (char**)envv, + .pgid = pgid, + .tcfd = tcfd, + }; + + if (!envv) + envv = environ; + sigcritical(SIG_REG_EXEC|SIG_REG_PROC|(tcfd>=0?SIG_REG_TERM:0)); + pid = clone(exec_process, stack+STACK_SIZE, CLONE_VM|CLONE_VFORK|SIGCHLD, &args); + if (pid == -1) + args.err = errno; + else if (args.err) + { + while (waitpid(pid, NULL, 0) == -1 && errno == EINTR); + pid = -1; + } + fork_cleanup(pid, pgid, args.err); + return pid; +} + +#elif _lib_posix_spawn > 1 /* reports underlying exec() errors */ +#define _fast_spawnveg 1 + +/* + * This version runs commands via posix_spawn(3) when possible + * for better performance than we'd get with fork(3). + */ + +#include + +static pid_t +spawnveg_fast(const char* path, char* const argv[], char* const envv[], pid_t pgid, int tcfd) +{ + int err; + short flags = 0; pid_t pid; posix_spawnattr_t attr; -#if _lib_posix_spawn_file_actions_addtcsetpgrp_np - posix_spawn_file_actions_t actions; -#else NOT_USED(tcfd); -#endif if (err = posix_spawnattr_init(&attr)) goto nope; -#if POSIX_SPAWN_SETSID +#ifdef POSIX_SPAWN_SETSID if (pgid == -1) flags |= POSIX_SPAWN_SETSID; #endif @@ -64,33 +210,14 @@ spawnveg(const char* path, char* const argv[], char* const envv[], pid_t pgid, i if (err = posix_spawnattr_setpgroup(&attr, pgid)) goto bad; } -#if _lib_posix_spawn_file_actions_addtcsetpgrp_np - if (tcfd >= 0) - { - if (err = posix_spawn_file_actions_init(&actions)) - goto bad; - if (err = posix_spawn_file_actions_addtcsetpgrp_np(&actions, tcfd)) - goto fail; - } - if (err = posix_spawn(&pid, path, (tcfd >= 0) ? &actions : NULL, &attr, argv, envv ? envv : environ)) -#else if (err = posix_spawn(&pid, path, NULL, &attr, argv, envv ? envv : environ)) -#endif { if ((err != EPERM) || (err = posix_spawn(&pid, path, NULL, NULL, argv, envv ? envv : environ))) - goto fail; + goto bad; } -#if _lib_posix_spawn_file_actions_addtcsetpgrp_np - if (tcfd >= 0) - posix_spawn_file_actions_destroy(&actions); -#endif posix_spawnattr_destroy(&attr); return pid; - fail: -#if _lib_posix_spawn_file_actions_addtcsetpgrp_np - if (tcfd >= 0) - posix_spawn_file_actions_destroy(&actions); -#endif + /* cleanup for different fail states */ bad: posix_spawnattr_destroy(&attr); nope: @@ -98,9 +225,8 @@ spawnveg(const char* path, char* const argv[], char* const envv[], pid_t pgid, i return -1; } -#else - -#if _lib_spawn_mode +#elif _lib_spawn_mode +#define _fast_spawnveg 1 #include @@ -111,8 +237,8 @@ spawnveg(const char* path, char* const argv[], char* const envv[], pid_t pgid, i #define P_DETACH _P_DETACH #endif -pid_t -spawnveg(const char* path, char* const argv[], char* const envv[], pid_t pgid, int tcfd) +static pid_t +spawnveg_fast(const char* path, char* const argv[], char* const envv[], pid_t pgid, int tcfd) { NOT_USED(tcfd); #if defined(P_DETACH) @@ -122,9 +248,8 @@ spawnveg(const char* path, char* const argv[], char* const envv[], pid_t pgid, i #endif } -#else - -#if _lib_spawn && _hdr_spawn && _mem_pgroup_inheritance +#elif _lib_spawn && _hdr_spawn && _mem_pgroup_inheritance +#define _fast_spawnveg 1 #include @@ -132,8 +257,8 @@ spawnveg(const char* path, char* const argv[], char* const envv[], pid_t pgid, i * MVS OpenEdition / z/OS fork+exec+(setpgid) */ -pid_t -spawnveg(const char* path, char* const argv[], char* const envv[], pid_t pgid, int tcfd) +static pid_t +spawnveg_fast(const char* path, char* const argv[], char* const envv[], pid_t pgid, int tcfd) { struct inheritance inherit; @@ -148,11 +273,8 @@ spawnveg(const char* path, char* const argv[], char* const envv[], pid_t pgid, i } #else - -#include -#include -#include -#include +#define _fast_spawnveg 0 +#endif /* _lib_posix_spawn */ #if _lib_spawnve && _hdr_process #include @@ -161,62 +283,53 @@ spawnveg(const char* path, char* const argv[], char* const envv[], pid_t pgid, i #endif #endif +#if _lib_pipe2 && O_cloexec +#define pipe(a) pipe2(a,O_cloexec) +#endif + /* * fork+exec+(setsid|setpgid) */ -pid_t -spawnveg(const char* path, char* const argv[], char* const envv[], pid_t pgid, int tcfd) +static pid_t +spawnveg_slow(const char* path, char* const argv[], char* const envv[], pid_t pgid, int tcfd) { int n; int m; pid_t pid; - pid_t rid; int err[2]; - NOT_USED(tcfd); if (!envv) envv = environ; #if _lib_spawnve - if (!pgid) + if (!pgid && tcfd < 0) return spawnve(path, argv, envv); #endif /* _lib_spawnve */ n = errno; if (pipe(err) < 0) err[0] = -1; +#if !(_lib_pipe2 && O_cloexec) else { fcntl(err[0], F_SETFD, FD_CLOEXEC); fcntl(err[1], F_SETFD, FD_CLOEXEC); } - sigcritical(SIG_REG_EXEC|SIG_REG_PROC); +#endif + sigcritical(SIG_REG_EXEC|SIG_REG_PROC|(tcfd>=0?SIG_REG_TERM:0)); pid = fork(); if (pid == -1) n = errno; else if (!pid) { - sigcritical(0); - if (pgid == -1) - setsid(); - else if (pgid) - { - m = 0; - if (pgid == 1 || pgid == -2 && (m = 1)) - pgid = getpid(); - if (setpgid(0, pgid) < 0 && errno == EPERM) - setpgid(pgid, 0); - if (m) - tcsetpgrp(2, pgid); - } + setup_child(pgid, tcfd); execve(path, argv, envv); if (err[0] != -1) { m = errno; write(err[1], &m, sizeof(m)); } - _exit(errno == ENOENT ? EXIT_NOTFOUND : EXIT_NOEXEC); + exit_child(); } - rid = pid; if (err[0] != -1) { close(err[1]); @@ -232,30 +345,31 @@ spawnveg(const char* path, char* const argv[], char* const envv[], pid_t pgid, i if (m) { while (waitpid(pid, &n, 0) && errno == EINTR); - rid = pid = -1; + pid = -1; n = m; } } close(err[0]); } - sigcritical(0); - if (pid != -1 && pgid > 0) - { - /* - * parent and child are in a race here - */ - - if (pgid == 1) - pgid = pid; - if (setpgid(pid, pgid) < 0 && pid != pgid && errno == EPERM) - setpgid(pid, pid); - } - errno = n; - return rid; + fork_cleanup(pid, pgid, n); + return pid; } -#endif +pid_t +spawnveg(const char* path, char* const argv[], char* const envv[], pid_t pgid, int tcfd) +{ +#if !_lib_clone + if(tcfd >= 0) + return spawnveg_slow(path, argv, envv, pgid, tcfd); #endif - +#if !_lib_clone && !defined(POSIX_SPAWN_SETSID) + if(pgid == -1) + return spawnveg_slow(path, argv, envv, pgid, tcfd); #endif +#if _fast_spawnveg + return spawnveg_fast(path, argv, envv, pgid, tcfd); +#else + return spawnveg_slow(path, argv, envv, pgid, tcfd); +#endif +} diff --git a/src/lib/libast/features/lib b/src/lib/libast/features/lib index 4a557acd539e..853f8ce1af0b 100644 --- a/src/lib/libast/features/lib +++ b/src/lib/libast/features/lib @@ -36,7 +36,7 @@ lib rand_r lib rewinddir,setlocale lib setpgrp,setpgrp2,setreuid,setuid lib socketpair -lib spawn,spawnve +lib clone,spawn,spawnve lib strcoll,strlcat,strlcpy lib strmode,strxfrm,strftime,symlink,sysconf,sysinfo lib telldir,tmpnam,tzset,universe,unlink,utime,wctype @@ -245,7 +245,6 @@ tst lib_posix_spawn unistd.h stdlib.h spawn.h -Dfork=______fork note{ posix_spaw _exit(n); } }end -lib posix_spawn_file_actions_addtcsetpgrp_np tst lib_spawn_mode unistd.h stdlib.h note{ first spawn arg is mode and it works }end execute{ #include @@ -390,7 +389,7 @@ tst - output{ int main(void) { - #if _lib_posix_spawn || _lib_spawn_mode || _lib_spawn && _hdr_spawn && _mem_pgroup_inheritance + #if _lib_posix_spawn || _lib_spawn_mode || _lib_spawn && _hdr_spawn && _mem_pgroup_inheritance || _lib_clone printf("#if !_AST_no_spawnveg\n"); printf("#define _use_spawnveg 1\n"); printf("#endif\n"); diff --git a/src/lib/libast/man/spawnveg.3 b/src/lib/libast/man/spawnveg.3 index 272443bac29f..996fed0693e8 100644 --- a/src/lib/libast/man/spawnveg.3 +++ b/src/lib/libast/man/spawnveg.3 @@ -52,9 +52,11 @@ combines and .IR setsid (2) into a single call. -If +If one of either .IR posix_spawn (3) -is available, that function is preferred over +or +.IR clone (2) +is available, those functions are preferred over .IR fork . .PP .LR command , @@ -81,30 +83,33 @@ The new process becomes a process group leader. The new process joins the process group .IR pgid . .PP -The -.L tcfd -argument is currently only used if the operating system supports the -.I posix_spawn_file_actions_addtcsetpgrp_np -function. When .L tcfd is .LR >=0 , spawnveg will set the controlling terminal for the new process to -.IR tcfd . +.IR tcfd , +and will set the terminal signals +.IR SIGTSTP , +.IR SIGTTIN , +and +.I SIGTTOU +to their defaults in the child process before +.I execve +is called. .SH CAVEATS -If the -.I posix_spawn_file_actions_addtcsetpgrp_np -function is not available, then -.L spawnveg -cannot reliably set the terminal process group. -As a result, it is incompatible with job control when used with terminals. -Additionally, if the -.L POSIX_SPAWN_SETSID -attribute is not supported, then +If +.I clone +and/or +.I POSIX_SPAWN_SETSID +aren't available, then .L spawnveg -cannot make the new process a session leader when using the -.I posix_spawn -API. +will fall back to using +.I fork +in cases where either +.I setsid +or +.IR tcsetpgrp (3) +is required. .SH "SEE ALSO" -fork(2), posix_spawn(3), exec(2), setpgid(2), setsid(2), spawnve(2) +fork(2), posix_spawn(3), clone(2), exec(2), setpgid(2), setsid(2), sigaction(2), spawnve(2), tcsetpgrp(3) diff --git a/src/lib/libast/misc/sigcrit.c b/src/lib/libast/misc/sigcrit.c index 6dc2227d1188..9561384125e9 100644 --- a/src/lib/libast/misc/sigcrit.c +++ b/src/lib/libast/misc/sigcrit.c @@ -91,8 +91,13 @@ sigcritical(int op) else { /* - * a vfork() may have intervened so we - * allow apparent nesting mismatches + * A vfork via clone(2) may have intervened so we + * allow apparent nesting mismatches. The child + * shares memory and will decrease the level to 0, + * which is then decreased again to -1 by the parent + * once the parent's execution resumes. + * (This assumes both the child and parent processes + * invoke sigcritical(0).) */ if (--level <= 0) From 18a30a514a1c67380f0715e7f98119124f790949 Mon Sep 17 00:00:00 2001 From: Johnothan King Date: Wed, 9 Jul 2025 10:48:35 -0700 Subject: [PATCH 2/5] clone(2) spawnveg: set args->env correctly --- src/lib/libast/comp/spawnveg.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/lib/libast/comp/spawnveg.c b/src/lib/libast/comp/spawnveg.c index 3cd76601c2d5..1265f51a0874 100644 --- a/src/lib/libast/comp/spawnveg.c +++ b/src/lib/libast/comp/spawnveg.c @@ -154,13 +154,11 @@ spawnveg_fast(const char* path, char* const argv[], char* const envv[], pid_t pg struct cargs args = { .path = path, .argv = (char**)argv, - .envv = (char**)envv, + .envv = (char**)(envv ? envv : environ), .pgid = pgid, .tcfd = tcfd, }; - if (!envv) - envv = environ; sigcritical(SIG_REG_EXEC|SIG_REG_PROC|(tcfd>=0?SIG_REG_TERM:0)); pid = clone(exec_process, stack+STACK_SIZE, CLONE_VM|CLONE_VFORK|SIGCHLD, &args); if (pid == -1) From d22d5ffe37e4121160b931975ff8b7a2498aefcf Mon Sep 17 00:00:00 2001 From: Johnothan King Date: Sat, 12 Jul 2025 01:30:20 -0700 Subject: [PATCH 3/5] Delete unused var n --- src/lib/libast/comp/spawnveg.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/lib/libast/comp/spawnveg.c b/src/lib/libast/comp/spawnveg.c index 1265f51a0874..f28fe9e94c81 100644 --- a/src/lib/libast/comp/spawnveg.c +++ b/src/lib/libast/comp/spawnveg.c @@ -148,7 +148,6 @@ static noreturn int exec_process(void *data) pid_t spawnveg_fast(const char* path, char* const argv[], char* const envv[], pid_t pgid, int tcfd) { - int n = errno; pid_t pid; char stack[STACK_SIZE]; struct cargs args = { From 644333c8102e59c06e7201649cce5b70837dae00 Mon Sep 17 00:00:00 2001 From: Johnothan King Date: Sat, 12 Jul 2025 01:33:13 -0700 Subject: [PATCH 4/5] Fix -Wunused-function warning --- src/lib/libast/comp/spawnveg.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/lib/libast/comp/spawnveg.c b/src/lib/libast/comp/spawnveg.c index f28fe9e94c81..1fbc4461e937 100644 --- a/src/lib/libast/comp/spawnveg.c +++ b/src/lib/libast/comp/spawnveg.c @@ -273,6 +273,8 @@ spawnveg_fast(const char* path, char* const argv[], char* const envv[], pid_t pg #define _fast_spawnveg 0 #endif /* _lib_posix_spawn */ +#if !_lib_clone + #if _lib_spawnve && _hdr_process #include #if defined(P_NOWAIT) || defined(_P_NOWAIT) @@ -352,6 +354,7 @@ spawnveg_slow(const char* path, char* const argv[], char* const envv[], pid_t pg return pid; } +#endif /* !_lib_clone */ pid_t spawnveg(const char* path, char* const argv[], char* const envv[], pid_t pgid, int tcfd) From 7fa6d4e6a626e950cc1dc491190f58b127c8b298 Mon Sep 17 00:00:00 2001 From: Johnothan King Date: Tue, 22 Jul 2025 06:55:41 -0700 Subject: [PATCH 5/5] spawnveg(): Increase stack size to fix __asan_handle_no_return warning --- src/lib/libast/comp/spawnveg.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/libast/comp/spawnveg.c b/src/lib/libast/comp/spawnveg.c index 1fbc4461e937..e68ad9db6f12 100644 --- a/src/lib/libast/comp/spawnveg.c +++ b/src/lib/libast/comp/spawnveg.c @@ -92,7 +92,7 @@ static noreturn void exit_child(void) #if _lib_clone #define _fast_spawnveg 1 -#define STACK_SIZE 1024*56 +#define STACK_SIZE 1024*512 #include /*