This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH] posix: Fix improper assert in Linux posix_spawn (BZ#22273)


As noted by Florian Weimer, current Linux posix_spawn implementation
can trigger an assert if the auxiliary process is terminated before
actually setting the err member:

    340   /* Child must set args.err to something non-negative - we rely on
    341      the parent and child sharing VM.  */
    342   args.err = -1;
    [...]
    362   new_pid = CLONE (__spawni_child, STACK (stack, stack_size), stack_size,
    363                    CLONE_VM | CLONE_VFORK | SIGCHLD, &args);
    364
    365   if (new_pid > 0)
    366     {
    367       ec = args.err;
    368       assert (ec >= 0);

Another possible issue is killing the child between setting the err and
actually calling execve.  In this case the process will not ran, but
posix_spawn also will not report any error:

    269
    270   args->err = 0;
    271   args->exec (args->file, args->argv, args->envp);

As suggested by Andreas Schwab, this patch removes the faulty assert
and also handles any signal that happens before fork and execve as the
spawn was successful (and thus relaying the handling to the caller to
figure this out).  Different than Florian, I can not see why using
atomics to set err would help here, essentially the code runs
sequentially (due CLONE_VFORK) and I think it would not be legal the
compiler evaluate ec without checking for new_pid result (thus there
is no need to compiler barrier).

Checked on x86_64-linux-gnu.

	[BZ #22273]
	* sysdeps/unix/sysv/linux/spawni.c (__spawnix): Handle the case where
	the auxiliary process is terminated by a signal before calling _exit
	or execve.
---
 ChangeLog                        |  6 ++++++
 sysdeps/unix/sysv/linux/spawni.c | 12 ++++++++++--
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/sysdeps/unix/sysv/linux/spawni.c b/sysdeps/unix/sysv/linux/spawni.c
index dea1650..d6acc1e 100644
--- a/sysdeps/unix/sysv/linux/spawni.c
+++ b/sysdeps/unix/sysv/linux/spawni.c
@@ -365,9 +365,17 @@ __spawnix (pid_t * pid, const char *file,
   if (new_pid > 0)
     {
       ec = args.err;
-      assert (ec >= 0);
       if (ec != 0)
-	  __waitpid (new_pid, NULL, 0);
+	{
+	  /* It handles the unlikely case where the auxiliary process is
+	     terminated by a signal before calling _exit or execve.  For
+	     this case the signal is relayed to the called, as the spawn
+	     was successful.  */
+	  int status;
+	  __waitpid (new_pid, &status, WNOHANG);
+	  if (WIFSIGNALED (status))
+	    ec = 0;
+	}
     }
   else
     ec = -new_pid;
-- 
2.7.4


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]