Skip to content

Commit

Permalink
Use vfork instead of posix_spawn
Browse files Browse the repository at this point in the history
WARNING: This is an experimental fix for the race conditions
         reported at ksh93#79 and att#468. There may
         be bugs that have not yet been caught.

This patch replaces usage of posix_spawn with vfork, since the
former is prone to race conditions.

src/lib/libast/features/lib,
src/lib/libast/comp/spawnveg.c:
- Remove the posix_spawn implementation of spawnveg.
- Set the terminal process group between fork/vfork and execve
  to fix a race condition.

src/cmd/ksh93/sh/xec.c:
- Set the terminal process group on error to fix a vfork(2) lockup
  and crash with fork(2); both occured after the to the spawnveg fix.
  • Loading branch information
JohnoKing committed Jul 20, 2020
1 parent bd88cc7 commit 11b0da6
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 166 deletions.
14 changes: 9 additions & 5 deletions src/cmd/ksh93/sh/xec.c
Original file line number Diff line number Diff line change
Expand Up @@ -3724,12 +3724,16 @@ static pid_t sh_ntfork(Shell_t *shp,const Shnode_t *t,char *argv[],int *jobid,in
fail:
if(jobfork && spawnpid<0)
job_fork(0);
if(spawnpid < 0) switch(errno=shp->path_err)
if(spawnpid < 0)
{
case ENOENT:
errormsg(SH_DICT,ERROR_system(ERROR_NOENT),e_found+4);
default:
errormsg(SH_DICT,ERROR_system(ERROR_NOEXEC),e_exec+4);
tcsetpgrp(2,job.curpgid);
switch(errno=shp->path_err)
{
case ENOENT:
errormsg(SH_DICT,ERROR_system(ERROR_NOENT),e_found+4);
default:
errormsg(SH_DICT,ERROR_system(ERROR_NOEXEC),e_exec+4);
}
}
}
else
Expand Down
50 changes: 4 additions & 46 deletions src/lib/libast/comp/spawnveg.c
Original file line number Diff line number Diff line change
Expand Up @@ -38,51 +38,6 @@ NoN(spawnveg)

#else

#if _lib_posix_spawn > 1 /* reports underlying exec() errors */

#include <spawn.h>
#include <error.h>
#include <wait.h>

pid_t
spawnveg(const char* path, char* const argv[], char* const envv[], pid_t pgid)
{
int err;
pid_t pid;
posix_spawnattr_t attr;

if (err = posix_spawnattr_init(&attr))
goto nope;
if (pgid)
{
if (pgid <= 1)
pgid = 0;
if (err = posix_spawnattr_setpgroup(&attr, pgid))
goto bad;
if (err = posix_spawnattr_setflags(&attr, POSIX_SPAWN_SETPGROUP))
goto bad;
}
if (err = posix_spawn(&pid, path, NiL, &attr, argv, envv ? envv : environ))
goto bad;
posix_spawnattr_destroy(&attr);
#if _lib_posix_spawn < 2
if (waitpid(pid, &err, WNOHANG|WNOWAIT) == pid && EXIT_STATUS(err) == 127)
{
while (waitpid(pid, NiL, 0) == -1 && errno == EINTR);
if (!access(path, X_OK))
errno = ENOEXEC;
pid = -1;
}
#endif
return pid;
bad:
posix_spawnattr_destroy(&attr);
nope:
errno = err;
return -1;
}

#else

#if _lib_spawn_mode

Expand Down Expand Up @@ -211,7 +166,11 @@ spawnveg(const char* path, char* const argv[], char* const envv[], pid_t pgid)
{
m = 0;
if (pgid == 1 || pgid == -2 && (m = 1))
{
pgid = getpid();
if (!m)
tcsetpgrp(2, pgid);
}
if (setpgid(0, pgid) < 0 && errno == EPERM)
setpgid(pgid, 0);
#if _lib_tcgetpgrp
Expand Down Expand Up @@ -291,6 +250,5 @@ spawnveg(const char* path, char* const argv[], char* const envv[], pid_t pgid)

#endif

#endif

#endif
116 changes: 1 addition & 115 deletions src/lib/libast/features/lib
Original file line number Diff line number Diff line change
Expand Up @@ -241,120 +241,6 @@ tst real_vfork note{ vfork child shares data with parent }end execute{
}
}end

tst lib_posix_spawn unistd.h stdlib.h spawn.h -Dfork=______fork note{ posix_spawn exists and it works and its worth using }end status{
#include <sys/types.h>
#include <sys/stat.h>
#include <sys/wait.h>
#include <spawn.h>
#include <signal.h>
#include <fcntl.h>
#include <string.h>
/* if it uses fork() why bother? */
#undef fork
pid_t fork _ARG_((void)) { NOTE("uses fork()"); return -1; }
pid_t _fork _ARG_((void)) { NOTE("uses _fork()"); return -1; }
pid_t __fork _ARG_((void)) { NOTE("uses __fork()"); return -1; }
int
main(argc, argv)
int argc;
char** argv;
{
char* s;
pid_t pid;
posix_spawnattr_t attr;
int n;
int status;
char* cmd[3];
char tmp[1024];
if (argv[1])
_exit(signal(SIGHUP, SIG_DFL) != SIG_IGN);
signal(SIGHUP, SIG_IGN);
if (posix_spawnattr_init(&attr))
{
NOTE("posix_spawnattr_init() FAILED");
_exit(0);
}
if (posix_spawnattr_setpgroup(&attr, 0))
{
NOTE("posix_spawnattr_setpgroup() FAILED");
_exit(0);
}
if (posix_spawnattr_setflags(&attr, POSIX_SPAWN_SETPGROUP))
{
NOTE("posix_spawnattr_setflags() FAILED");
_exit(0);
}
/* first try an a.out and verify that SIGHUP is ignored */
cmd[0] = argv[0];
cmd[1] = "test";
cmd[2] = 0;
if (posix_spawn(&pid, cmd[0], 0, &attr, cmd, 0))
{
NOTE("posix_spawn() FAILED");
_exit(0);
}
status = 1;
if (wait(&status) < 0)
{
NOTE("wait() FAILED");
_exit(0);
}
if (status != 0)
{
NOTE("SIGHUP ignored in parent not ignored in child");
_exit(0);
}
/* must return exec-type errors or its useless to us *unless* there is no [v]fork() */
n = strlen(cmd[0]);
if (n >= (sizeof(tmp) - 3))
{
NOTE("test executable path too long");
_exit(0);
}
strcpy(tmp, cmd[0]);
tmp[n] = '.';
tmp[n+1] = 's';
tmp[n+2] = 'h';
tmp[n+3] = 0;
if ((n = open(tmp, O_CREAT|O_WRONLY, S_IRWXU|S_IRWXG|S_IRWXO)) < 0 ||
chmod(tmp, S_IRWXU|S_IRWXG|S_IRWXO) < 0 ||
write(n, "exit 99\n", 8) != 8 ||
close(n) < 0)
{
NOTE("test script create FAILED");
_exit(0);
}
cmd[0] = tmp;
n = 0; /* 0 means reject */
pid = -1;
if (posix_spawn(&pid, cmd[0], 0, &attr, cmd, 0))
{
n = 2;
NOTE("ENOEXEC produces posix_spawn() error (BEST)");
}
else if (pid == -1)
NOTE("ENOEXEC returns pid == -1");
else if (wait(&status) != pid)
NOTE("ENOEXEC produces no child process");
else if (!WIFEXITED(status))
NOTE("ENOEXEC produces signal exit");
else
{
status = WEXITSTATUS(status);
if (status == 127)
{
n = 1;
NOTE("ENOEXEC produces exit status 127 (GOOD)");
}
else if (status == 99)
NOTE("ENOEXEC invokes sh");
else if (status == 0)
NOTE("ENOEXEC reports no error");
}
_exit(n);
}
}end

tst lib_spawn_mode unistd.h stdlib.h note{ first spawn arg is mode and it works }end execute{
#include <signal.h>
#include <process.h>
Expand Down Expand Up @@ -689,7 +575,7 @@ tst - output{
printf("\n");
#endif

#if _lib_spawnveg || _lib_posix_spawn || _lib_spawn_mode || _lib_spawn && _hdr_spawn && _mem_pgroup_inheritance || _lib_vfork && _real_vfork
#if _lib_spawnveg || _lib_spawn_mode || _lib_spawn && _hdr_spawn && _mem_pgroup_inheritance || _lib_vfork && _real_vfork
printf("#if !_AST_no_spawnveg\n");
printf("#define _use_spawnveg 1\n");
printf("#endif\n");
Expand Down

0 comments on commit 11b0da6

Please sign in to comment.