Overhaul spawnveg to use clone(2) and fix many related spawning bugs

NEWS

@@ -12,7 +12,17 @@ Uppercase BUG_* IDs are shell bug IDs as used by the Modernish shell library.
   directory with fchdir(2).
 
 - [v1.1] Fixed a file descriptor leak that could occur on systems lacking a
-  useable implementation of arc4random(3).
+  usable implementation of arc4random(3).
+
+- 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).
 
 2026-02-04:
 

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();
 }
 
 /*

src/cmd/ksh93/sh/xec.c

@@ -44,11 +44,6 @@
 #   include <sys/resource.h>
 #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 */
@@ -1436,11 +1431,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)
@@ -3324,9 +3315,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)
 {
@@ -3337,9 +3331,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 */
@@ -3391,22 +3382,13 @@ 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)
 				grp = 1;
 			else
 				grp = job.curpgid;
 		}
-#endif /* _use_ntfork_tcpgrp */
 
 		sfsync(NULL);
 		sigreset(0);	/* set signals to ignore */
@@ -3419,35 +3401,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();
 	}
@@ -3456,17 +3422,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)
@@ -3476,7 +3431,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);

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"))"

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)"
 
 # ======