public inbox for gcc@gcc.gnu.org
 help / color / mirror / Atom feed
* libiberty/pex-unix vfork abuse?
@ 2007-12-07 15:08 Dave Korn
  2007-12-07 16:59 ` Ian Lance Taylor
  0 siblings, 1 reply; 26+ messages in thread
From: Dave Korn @ 2007-12-07 15:08 UTC (permalink / raw)
  To: gcc


    Hey all,

  This is what posix says about vfork:

http://www.opengroup.org/onlinepubs/000095399/functions/vfork.html
"The vfork() function shall be equivalent to fork(), except that the behavior
is undefined if the process created by vfork() either modifies any data other
than a variable of type pid_t used to store the return value from vfork(), or
returns from the function in which vfork() was called, or calls any other
function before successfully calling _exit() or one of the exec family of
functions."

  This is how pex-unix.c uses vfork:

static long
pex_unix_exec_child (struct pex_obj *obj, int flags, const char *executable,
		     char * const * argv, char * const * env,
                     int in, int out, int errdes,
		     int toclose, const char **errmsg, int *err)
{
  pid_t pid;

  /* We declare these to be volatile to avoid warnings from gcc about
     them being clobbered by vfork.  */
  volatile int sleep_interval;
  volatile int retries;

  sleep_interval = 1;
  pid = -1;
  for (retries = 0; retries < 4; ++retries)
    {
      pid = vfork ();
      if (pid >= 0)
	break;
      sleep (sleep_interval);
      sleep_interval *= 2;
    }

  switch (pid)
    {
    case -1:
      *err = errno;
      *errmsg = VFORK_STRING;
      return -1;

    case 0:
      /* Child process.  */
      if (in != STDIN_FILE_NO)
	{
	  if (dup2 (in, STDIN_FILE_NO) < 0)
	    pex_child_error (obj, executable, "dup2", errno);
	  if (close (in) < 0)
	    pex_child_error (obj, executable, "close", errno);
	}
      if (out != STDOUT_FILE_NO)
	{
	  if (dup2 (out, STDOUT_FILE_NO) < 0)
	    pex_child_error (obj, executable, "dup2", errno);
	  if (close (out) < 0)
	    pex_child_error (obj, executable, "close", errno);
	}
      if (errdes != STDERR_FILE_NO)
	{
	  if (dup2 (errdes, STDERR_FILE_NO) < 0)
	    pex_child_error (obj, executable, "dup2", errno);
	  if (close (errdes) < 0)
	    pex_child_error (obj, executable, "close", errno);
	}
      if (toclose >= 0)
	{
	  if (close (toclose) < 0)
	    pex_child_error (obj, executable, "close", errno);
	}
      if ((flags & PEX_STDERR_TO_STDOUT) != 0)
	{
	  if (dup2 (STDOUT_FILE_NO, STDERR_FILE_NO) < 0)
	    pex_child_error (obj, executable, "dup2", errno);
	}

      if (env)
        environ = (char**) env;

      if ((flags & PEX_SEARCH) != 0)
	{
	  execvp (executable, argv);
	  pex_child_error (obj, executable, "execvp", errno);
	}
      else
	{
	  execv (executable, argv);
	  pex_child_error (obj, executable, "execv", errno);
	}



  Note the several calls to dup2() and close(), which seem to me to be "calls
[to] any other function", and the setting of environ, which seem to me to be
modification of "any data other than a variable of type pid_t used to store
the return value from vfork()".  The comment on pex_child_error (which uses
write() to do output) gives a hint at the thinking here:


/* Report an error from a child process.  We don't use stdio routines,
   because we might be here due to a vfork call.  */

static void
pex_child_error (struct pex_obj *obj, const char *executable,
		 const char *errmsg, int err)
{
#define writeerr(s) (void) write (STDERR_FILE_NO, s, strlen (s))
  writeerr (obj->pname);


  But I don't see any reason to assume the restriction only applies to f*()
stdio functions, in fact by my reading I don't think you're [technically] even
allowed to call a pure const inline function that's part of your own code.  (I
assume that that would in fact work ok in practice at least most of the time).

  Are we ok here?  This code seems like it's doing the wrong thing to me.  As
far as I can tell, we only get away with this in cygwin because of paranoid
defensive programming that backs up the fd table before running the vfork'd
child's code in the parent's context up to the first exec*() call, and then
restores it afterward, but I'm fairly sure that this implementation will still
overwrite the parent's environment.... which could well be Not A Good Thing!


    cheers,
      DaveK
-- 
Can't think of a witty .sigline today....

^ permalink raw reply	[flat|nested] 26+ messages in thread
* RE: libiberty/pex-unix vfork abuse?
@ 2007-12-07 17:24 J.C. Pizarro
  2007-12-07 17:42 ` Dave Korn
  0 siblings, 1 reply; 26+ messages in thread
From: J.C. Pizarro @ 2007-12-07 17:24 UTC (permalink / raw)
  To: Dave Korn, Ian Lance Taylor, gcc

On 2007/12/07, "Dave Korn" <dave.korn@artimi.com> wrote:
> > On the other hand, the setting of environ is very dubious and is
> > likely to break on real systems.  The code should be changed to call
> > execve instead.  Unfortunately there is no standard execvpe function.
> > Fortunately gcc never uses the variant which sets environ.  Offhand
> > I'm not sure what does.
>
>   Perhaps we could work around this case by setting environ in the parent
> before the vfork call and restoring it afterward, but we'd need kind of
> serialisation there, and I don't know how to do a critical section using
> pthreads/posix.

You can do a critical section mainly between processes using system calls of
IPC synchronization like filelocks, RPCs, shared memory with mmap and
mutexes/semaphores, messages passing through pipes as tunnels, MPI, etc.

Now well, a critical section between multithreaded processes are complicated.

   J.C.Pizarro

^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: libiberty/pex-unix vfork abuse?
@ 2007-12-07 19:38 Ross Ridge
  0 siblings, 0 replies; 26+ messages in thread
From: Ross Ridge @ 2007-12-07 19:38 UTC (permalink / raw)
  To: gcc

Dave Korn writes:
>  Perhaps we could work around this case by setting environ in the parent
> before the vfork call and restoring it afterward, but we'd need kind of
> serialisation there, and I don't know how to do a critical section using
> pthreads/posix.

A simple solution would be to call fork() instead of vfork() when changing
the environment.

					Ross Ridge

^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2007-12-10 23:28 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-12-07 15:08 libiberty/pex-unix vfork abuse? Dave Korn
2007-12-07 16:59 ` Ian Lance Taylor
2007-12-07 17:09   ` Dave Korn
2007-12-07 18:40     ` Ian Lance Taylor
2007-12-07 20:55     ` Andreas Schwab
2007-12-10 19:19       ` Dave Korn
2007-12-10 19:22         ` Joe Buck
2007-12-10 19:37           ` Daniel Jacobowitz
2007-12-10 20:24             ` Joe Buck
2007-12-10 22:44             ` Andreas Schwab
2007-12-10 23:10               ` Daniel Jacobowitz
2007-12-10 21:05           ` Ian Lance Taylor
2007-12-10 22:56           ` Andreas Schwab
2007-12-10 20:01         ` Andreas Schwab
2007-12-10 20:06           ` Brian Dessent
2007-12-11  3:31             ` Dave Korn
2007-12-07 17:24 J.C. Pizarro
2007-12-07 17:42 ` Dave Korn
2007-12-07 17:50   ` Joe Buck
2007-12-07 18:09     ` J.C. Pizarro
2007-12-07 18:14       ` Andrew Haley
2007-12-07 18:28       ` Dave Korn
2007-12-07 18:47         ` J.C. Pizarro
2007-12-07 18:31       ` Diego Novillo
2007-12-07 22:14       ` Gabriel Dos Reis
2007-12-07 19:38 Ross Ridge

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).