This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
[PATCH] posix: Fix improper assert in Linux posix_spawn (BZ#22273)
- From: Adhemerval Zanella <adhemerval dot zanella at linaro dot org>
- To: libc-alpha at sourceware dot org
- Date: Thu, 12 Oct 2017 15:39:22 -0300
- Subject: [PATCH] posix: Fix improper assert in Linux posix_spawn (BZ#22273)
- Authentication-results: sourceware.org; auth=none
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