[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#882794: glibc: Please backport fix for assertion failure in posix_spawn()



Source: glibc
Version: 2.24-11+deb9u1
Severity: important
Tags: patch upstream

Hello!

With glibc 2.25, an upstream change was introduced with causes an
assertion failure on current versions of QEMU:

dpkg: warning: ignoring pre-dependency problem!
Preparing to unpack .../archives/bash_4.4-5_m68k.deb ...
preinst: ../sysdeps/unix/sysv/linux/spawni.c:366: __spawnix: Assertion `ec >= 0' failed.
qemu: uncaught target signal 6 (Aborted) - core dumped
dpkg: error processing archive /var/cache/apt/archives/bash_4.4-5_m68k.deb (--unpack):
 new bash package pre-installation script subprocess was killed by signal (Aborted)
 Selecting previously unselected package bsdutils.
 dpkg: regarding .../bsdutils_1%3a2.30.2-0.1_m68k.deb containing bsdutils, pre-dependency problem:
  bsdutils pre-depends on libsystemd0
    libsystemd0 is not installed.

This was introduced with [1] and reported in [2]. A QEMU bug report
has also been opened [3]. I'm currently rebuilding glibc for m68k with
the attached patch which should fix the issue. Would be great if it
could be included in one of the next uploads provided that it fixes
the issue which I am going to find out soon.

Adrian

> [1] https://sourceware.org/git/?p=glibc.git;a=commitdiff;h=4b4d4056bb154603f36c6f8845757c1012758158;hp=8d3bd947483f50b57aee7c35c07dc1927d6e8a27
> [2] https://sourceware.org/bugzilla/show_bug.cgi?id=22273
> [3] https://bugs.launchpad.net/qemu/+bug/1673976

--
  .''`.  John Paul Adrian Glaubitz
 : :' :  Debian Developer - glaubitz@debian.org
 `. `'   Freie Universitaet Berlin - glaubitz@physik.fu-berlin.de
   `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913
Description: Fix improper assert in Linux posix_spawn (BZ#22273)
 Fixes assertion failure on qemu-user.
 .
Origin: upstream
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=22273
Last-Update: 2017-11-26

--- glibc-2.25.orig/sysdeps/unix/sysv/linux/spawni.c
+++ glibc-2.25/sysdeps/unix/sysv/linux/spawni.c
@@ -17,7 +17,6 @@
    <http://www.gnu.org/licenses/>.  */
 
 #include <spawn.h>
-#include <assert.h>
 #include <fcntl.h>
 #include <paths.h>
 #include <string.h>
@@ -266,7 +265,6 @@ __spawni_child (void *arguments)
   __sigprocmask (SIG_SETMASK, (attr->__flags & POSIX_SPAWN_SETSIGMASK)
 		 ? &attr->__ss : &args->oldmask, 0);
 
-  args->err = 0;
   args->exec (args->file, args->argv, args->envp);
 
   /* This is compatibility function required to enable posix_spawn run
@@ -337,7 +335,7 @@ __spawnix (pid_t * pid, const char *file
 
   /* Child must set args.err to something non-negative - we rely on
      the parent and child sharing VM.  */
-  args.err = -1;
+  args.err = 0;
   args.file = file;
   args.exec = exec;
   args.fa = file_actions;
@@ -360,12 +358,26 @@ __spawnix (pid_t * pid, const char *file
   new_pid = CLONE (__spawni_child, STACK (stack, stack_size), stack_size,
 		   CLONE_VM | CLONE_VFORK | SIGCHLD, &args);
 
+  /* It needs to collect the case where the auxiliary process was created
+     but failed to execute the file (due either any preparation step or
+     for execve itself).  */
   if (new_pid > 0)
     {
+      /* Also, it handles the unlikely case where the auxiliary process was
+	 terminated before calling execve as if it was successfully.  The
+	 args.err is set to 0 as default and changed to a positive value
+	 only in case of failure, so in case of premature termination
+	 due a signal args.err will remain zeroed and it will be up to
+	 caller to actually collect it.  */
       ec = args.err;
-      assert (ec >= 0);
-      if (ec != 0)
-	  __waitpid (new_pid, NULL, 0);
+      if (ec > 0)
+	/* There still an unlikely case where the child is cancelled after
+	   setting args.err, due to a positive error value.  Also due a
+	   possible pid reuse race (where the kernel allocated the same pid
+	   to unrelated process) we need not to undefinitely hang expecting
+	   an invalid pid.  In both cases an error is returned to the
+	   caller.  */
+	__waitpid (new_pid, NULL, WNOHANG);
     }
   else
     ec = -new_pid;

Reply to: