[v3,2/2] posix: Use posix_spawn on system

Message ID de07a783-eb86-960e-1141-3ef18be27a85@linaro.org
State Dropped
Headers

Commit Message

Adhemerval Zanella Nov. 30, 2018, 6:25 p.m. UTC
  On 30/11/2018 13:21, Florian Weimer wrote:
> * Adhemerval Zanella:
> 
>> On 29/11/2018 15:37, Florian Weimer wrote:
>>> * Adhemerval Zanella:
>>>
>>>> +/* We have to and actually can handle cancelable system().  The big
>>>> +   problem: we have to kill the child process if necessary.  To do
>>>> +   this a cleanup handler has to be registered and it has to be able
>>>> +   to find the PID of the child.  The main problem is to reliable have
>>>> +   the PID when needed.  It is not necessary for the parent thread to
>>>> +   return.  It might still be in the kernel when the cancellation
>>>> +   request comes.  Therefore we have to use the clone() calls ability
>>>> +   to have the kernel write the PID into the user-level variable.  */
>>>
>>> This comment does not look relevant to me anymore.
>>
>> I think it still worth to mention glibc system aims to be thread-safe,
>> which requires restore the signal dispositions for SIGINT and SIGQUIT 
>> correctly and to deal with cancellation by terminating the child process.
> 
>> +/* This system implementation aims to be thread-safe, which requires restore
>> +   the signal dispositions for SIGINT and SIGQUIT correctly and to deal with
>> +   cancellation by terminating the child process.  */
> 
> I don't think you restore SIGINT and SIGQUIT correctly for concurrent
> system calls.  This is what the ADD_REF code in the old version
> attempted to do.

It is not strictly incorrect, although Linux sigaction is not really 
thread-safe (due the copy in/out kernel sigaction structure).  And it
is indeed not optional, and I agree that relying on this benign data race
behaviour is not correct. Below it is an updated patch with ref counter
reinstated.

---
  

Comments

Florian Weimer Nov. 30, 2018, 6:34 p.m. UTC | #1
* Adhemerval Zanella:

> On 30/11/2018 13:21, Florian Weimer wrote:
>> * Adhemerval Zanella:
>> 
>>> On 29/11/2018 15:37, Florian Weimer wrote:
>>>> * Adhemerval Zanella:
>>>>
>>>>> +/* We have to and actually can handle cancelable system().  The big
>>>>> +   problem: we have to kill the child process if necessary.  To do
>>>>> +   this a cleanup handler has to be registered and it has to be able
>>>>> +   to find the PID of the child.  The main problem is to reliable have
>>>>> +   the PID when needed.  It is not necessary for the parent thread to
>>>>> +   return.  It might still be in the kernel when the cancellation
>>>>> +   request comes.  Therefore we have to use the clone() calls ability
>>>>> +   to have the kernel write the PID into the user-level variable.  */
>>>>
>>>> This comment does not look relevant to me anymore.
>>>
>>> I think it still worth to mention glibc system aims to be thread-safe,
>>> which requires restore the signal dispositions for SIGINT and SIGQUIT 
>>> correctly and to deal with cancellation by terminating the child process.
>> 
>>> +/* This system implementation aims to be thread-safe, which requires restore
>>> +   the signal dispositions for SIGINT and SIGQUIT correctly and to deal with
>>> +   cancellation by terminating the child process.  */
>> 
>> I don't think you restore SIGINT and SIGQUIT correctly for concurrent
>> system calls.  This is what the ADD_REF code in the old version
>> attempted to do.
>
> It is not strictly incorrect, although Linux sigaction is not really 
> thread-safe (due the copy in/out kernel sigaction structure).  And it
> is indeed not optional, and I agree that relying on this benign data race
> behaviour is not correct. Below it is an updated patch with ref counter
> reinstated.

It's not about the data race, it is about the higher-level race
condition.  The problem is that the first thread to enter system and
capture the original signal state may not be the last to leave system
and restore things.

> +static void
> +cancel_handler (void *arg)
> +{
> +  struct cancel_handler_args *args = (struct cancel_handler_args *) (arg);
> +
> +  __kill_noerrno (args->pid, SIGKILL);
> +
> +  TEMP_FAILURE_RETRY (__waitpid (args->pid, NULL, 0));

One last question (I promise): Should this be the nocancel variant?

Thanks,
Florian
  
Adhemerval Zanella Nov. 30, 2018, 7:32 p.m. UTC | #2
On 30/11/2018 16:34, Florian Weimer wrote:
> * Adhemerval Zanella:
> 
>> On 30/11/2018 13:21, Florian Weimer wrote:
>>> * Adhemerval Zanella:
>>>
>>>> On 29/11/2018 15:37, Florian Weimer wrote:
>>>>> * Adhemerval Zanella:
>>>>>
>>>>>> +/* We have to and actually can handle cancelable system().  The big
>>>>>> +   problem: we have to kill the child process if necessary.  To do
>>>>>> +   this a cleanup handler has to be registered and it has to be able
>>>>>> +   to find the PID of the child.  The main problem is to reliable have
>>>>>> +   the PID when needed.  It is not necessary for the parent thread to
>>>>>> +   return.  It might still be in the kernel when the cancellation
>>>>>> +   request comes.  Therefore we have to use the clone() calls ability
>>>>>> +   to have the kernel write the PID into the user-level variable.  */
>>>>>
>>>>> This comment does not look relevant to me anymore.
>>>>
>>>> I think it still worth to mention glibc system aims to be thread-safe,
>>>> which requires restore the signal dispositions for SIGINT and SIGQUIT 
>>>> correctly and to deal with cancellation by terminating the child process.
>>>
>>>> +/* This system implementation aims to be thread-safe, which requires restore
>>>> +   the signal dispositions for SIGINT and SIGQUIT correctly and to deal with
>>>> +   cancellation by terminating the child process.  */
>>>
>>> I don't think you restore SIGINT and SIGQUIT correctly for concurrent
>>> system calls.  This is what the ADD_REF code in the old version
>>> attempted to do.
>>
>> It is not strictly incorrect, although Linux sigaction is not really 
>> thread-safe (due the copy in/out kernel sigaction structure).  And it
>> is indeed not optional, and I agree that relying on this benign data race
>> behaviour is not correct. Below it is an updated patch with ref counter
>> reinstated.
> 
> It's not about the data race, it is about the higher-level race
> condition.  The problem is that the first thread to enter system and
> capture the original signal state may not be the last to leave system
> and restore things.

Even with current implementation the program would need to coordinate 
sigaction with system, so I am not sure which would be best option 
(try to get some sanity on concurrent system or let the program
handle it).

> 
>> +static void
>> +cancel_handler (void *arg)
>> +{
>> +  struct cancel_handler_args *args = (struct cancel_handler_args *) (arg);
>> +
>> +  __kill_noerrno (args->pid, SIGKILL);
>> +
>> +  TEMP_FAILURE_RETRY (__waitpid (args->pid, NULL, 0));
> 
> One last question (I promise): Should this be the nocancel variant?

The cancel_handler will be called from within the sigcancel_handler
(through unwind mechanism) and SIGCANCEL is not installed SA_NODEFER,
so I can't see how another cancellation can act. However for consistency
using __waitpid_nocancel does make sense. I changed it locally.
  
Florian Weimer Nov. 30, 2018, 7:35 p.m. UTC | #3
* Adhemerval Zanella:

> On 30/11/2018 16:34, Florian Weimer wrote:
>> * Adhemerval Zanella:
>> 
>>> On 30/11/2018 13:21, Florian Weimer wrote:
>>>> * Adhemerval Zanella:
>>>>
>>>>> On 29/11/2018 15:37, Florian Weimer wrote:
>>>>>> * Adhemerval Zanella:
>>>>>>
>>>>>>> +/* We have to and actually can handle cancelable system().  The big
>>>>>>> +   problem: we have to kill the child process if necessary.  To do
>>>>>>> +   this a cleanup handler has to be registered and it has to be able
>>>>>>> +   to find the PID of the child.  The main problem is to reliable have
>>>>>>> +   the PID when needed.  It is not necessary for the parent thread to
>>>>>>> +   return.  It might still be in the kernel when the cancellation
>>>>>>> +   request comes.  Therefore we have to use the clone() calls ability
>>>>>>> +   to have the kernel write the PID into the user-level variable.  */
>>>>>>
>>>>>> This comment does not look relevant to me anymore.
>>>>>
>>>>> I think it still worth to mention glibc system aims to be thread-safe,
>>>>> which requires restore the signal dispositions for SIGINT and SIGQUIT 
>>>>> correctly and to deal with cancellation by terminating the child process.
>>>>
>>>>> +/* This system implementation aims to be thread-safe, which requires restore
>>>>> +   the signal dispositions for SIGINT and SIGQUIT correctly and to deal with
>>>>> +   cancellation by terminating the child process.  */
>>>>
>>>> I don't think you restore SIGINT and SIGQUIT correctly for concurrent
>>>> system calls.  This is what the ADD_REF code in the old version
>>>> attempted to do.
>>>
>>> It is not strictly incorrect, although Linux sigaction is not really 
>>> thread-safe (due the copy in/out kernel sigaction structure).  And it
>>> is indeed not optional, and I agree that relying on this benign data race
>>> behaviour is not correct. Below it is an updated patch with ref counter
>>> reinstated.
>> 
>> It's not about the data race, it is about the higher-level race
>> condition.  The problem is that the first thread to enter system and
>> capture the original signal state may not be the last to leave system
>> and restore things.
>
> Even with current implementation the program would need to coordinate 
> sigaction with system, so I am not sure which would be best option 
> (try to get some sanity on concurrent system or let the program
> handle it).

The common case where sigaction is called once at process start would be
okay, though.

>>> +static void
>>> +cancel_handler (void *arg)
>>> +{
>>> +  struct cancel_handler_args *args = (struct cancel_handler_args *) (arg);
>>> +
>>> +  __kill_noerrno (args->pid, SIGKILL);
>>> +
>>> +  TEMP_FAILURE_RETRY (__waitpid (args->pid, NULL, 0));
>> 
>> One last question (I promise): Should this be the nocancel variant?
>
> The cancel_handler will be called from within the sigcancel_handler
> (through unwind mechanism) and SIGCANCEL is not installed SA_NODEFER,
> so I can't see how another cancellation can act. However for consistency
> using __waitpid_nocancel does make sense. I changed it locally.

Thanks!

I don't have any further comments on your patch.  It is probably okay to
commit.

Florian
  

Patch

diff --git a/sysdeps/generic/not-errno.h b/sysdeps/generic/not-errno.h
index 93617a3266..0fd66b5c5e 100644
--- a/sysdeps/generic/not-errno.h
+++ b/sysdeps/generic/not-errno.h
@@ -17,3 +17,5 @@ 
    <http://www.gnu.org/licenses/>.  */
 
 extern __typeof (__access) __access_noerrno attribute_hidden;
+
+extern __typeof (__kill) __kill_noerrno attribute_hidden;
diff --git a/sysdeps/posix/system.c b/sysdeps/posix/system.c
index d7594436ed..76e67373dc 100644
--- a/sysdeps/posix/system.c
+++ b/sysdeps/posix/system.c
@@ -17,20 +17,35 @@ 
 
 #include <errno.h>
 #include <signal.h>
-#include <stddef.h>
 #include <stdlib.h>
 #include <unistd.h>
+#include <sigsetops.h>
+#include <spawn.h>
+#include <pthread.h>
 #include <sys/types.h>
 #include <sys/wait.h>
-#include <libc-lock.h>
-#include <sysdep-cancel.h>
-#include <sigsetops.h>
+#include <stdio.h>
 
+#include <libc-lock.h>
+#include <not-errno.h>
+#include <internal-signals.h>
 
 #define	SHELL_PATH	"/bin/sh"	/* Path of the shell.  */
 #define	SHELL_NAME	"sh"		/* Name to give it.  */
 
 
+/* This system implementation aims to be thread-safe, which requires to
+   restore the signal dispositions for SIGINT and SIGQUIT correctly and to
+   deal with cancellation by terminating the child process.
+
+   The signal disposition restoration on the single-thread case is
+   straighfoward.  For multithreaded case, a reference-counter with a lock
+   is used, so the first thread will set the SIGINT/SIGQUIT dispositions and
+   last thread will restore them.
+
+   Cancellation handling is done with thread cancellation clean-up handlers
+   on waitpid call.  */
+
 #ifdef _LIBC_REENTRANT
 static struct sigaction intr, quit;
 static int sa_refcntr;
@@ -50,17 +65,45 @@  __libc_lock_define_initialized (static, lock);
 #endif
 
 
+#if defined(_LIBC_REENTRANT) && defined(SIGCANCEL)
+struct cancel_handler_args
+{
+  struct sigaction *quit;
+  struct sigaction *intr;
+  pid_t pid;
+};
+
+static void
+cancel_handler (void *arg)
+{
+  struct cancel_handler_args *args = (struct cancel_handler_args *) (arg);
+
+  __kill_noerrno (args->pid, SIGKILL);
+
+  TEMP_FAILURE_RETRY (__waitpid (args->pid, NULL, 0));
+
+  DO_LOCK ();
+  if (SUB_REF () == 0)
+    {
+      __sigaction (SIGQUIT, args->quit, NULL);
+      __sigaction (SIGINT, args->intr, NULL);
+    }
+  DO_UNLOCK ();
+}
+#endif
+
 /* Execute LINE as a shell command, returning its status.  */
 static int
 do_system (const char *line)
 {
-  int status, save;
+  int status;
   pid_t pid;
   struct sigaction sa;
 #ifndef _LIBC_REENTRANT
   struct sigaction intr, quit;
 #endif
   sigset_t omask;
+  sigset_t reset;
 
   sa.sa_handler = SIG_IGN;
   sa.sa_flags = 0;
@@ -69,105 +112,72 @@  do_system (const char *line)
   DO_LOCK ();
   if (ADD_REF () == 0)
     {
-      if (__sigaction (SIGINT, &sa, &intr) < 0)
-	{
-	  (void) SUB_REF ();
-	  goto out;
-	}
-      if (__sigaction (SIGQUIT, &sa, &quit) < 0)
-	{
-	  save = errno;
-	  (void) SUB_REF ();
-	  goto out_restore_sigint;
-	}
+      /* sigaction can not fail with SIGINT/SIGQUIT used with SIG_IGN.  */
+      __sigaction (SIGINT, &sa, &intr);
+      __sigaction (SIGQUIT, &sa, &quit);
     }
   DO_UNLOCK ();
 
-  /* We reuse the bitmap in the 'sa' structure.  */
   __sigaddset (&sa.sa_mask, SIGCHLD);
-  save = errno;
-  if (__sigprocmask (SIG_BLOCK, &sa.sa_mask, &omask) < 0)
+  /* sigprocmask can not fail with SIG_BLOCK used with valid input
+     arguments.  */
+  __sigprocmask (SIG_BLOCK, &sa.sa_mask, &omask);
+
+  __sigemptyset (&reset);
+  if (intr.sa_handler != SIG_IGN)
+    __sigaddset(&reset, SIGINT);
+  if (quit.sa_handler != SIG_IGN)
+    __sigaddset(&reset, SIGQUIT);
+
+  posix_spawnattr_t spawn_attr;
+  /* None of the posix_spawnattr_* function returns an error, including
+     posix_spawnattr_setflags for the follow specific usage (using valid
+     flags).  */
+  __posix_spawnattr_init (&spawn_attr);
+  __posix_spawnattr_setsigmask (&spawn_attr, &omask);
+  __posix_spawnattr_setsigdefault (&spawn_attr, &reset);
+  __posix_spawnattr_setflags (&spawn_attr,
+			      POSIX_SPAWN_SETSIGDEF | POSIX_SPAWN_SETSIGMASK);
+
+  status = __posix_spawn (&pid, SHELL_PATH, 0, &spawn_attr,
+			  (char *const[]){ (char*) SHELL_NAME,
+					   (char*) "-c",
+					   (char *) line, NULL },
+			  __environ);
+  __posix_spawnattr_destroy (&spawn_attr);
+
+  if (status == 0)
     {
-#ifndef _LIBC
-      if (errno == ENOSYS)
-	__set_errno (save);
-      else
-#endif
-	{
-	  DO_LOCK ();
-	  if (SUB_REF () == 0)
-	    {
-	      save = errno;
-	      (void) __sigaction (SIGQUIT, &quit, (struct sigaction *) NULL);
-	    out_restore_sigint:
-	      (void) __sigaction (SIGINT, &intr, (struct sigaction *) NULL);
-	      __set_errno (save);
-	    }
-	out:
-	  DO_UNLOCK ();
-	  return -1;
-	}
-    }
-
-#ifdef CLEANUP_HANDLER
-  CLEANUP_HANDLER;
-#endif
-
-#ifdef FORK
-  pid = FORK ();
-#else
-  pid = __fork ();
+      /* Cancellation results in cleanup handlers running as exceptions in
+	 the block where they were installed, so it is safe to reference
+	 stack variable allocate in the broader scope.  */
+#if defined(_LIBC_REENTRANT) && defined(SIGCANCEL)
+      struct cancel_handler_args cancel_args =
+      {
+	.quit = &quit,
+	.intr = &intr,
+	.pid = pid
+      };
+      __libc_cleanup_region_start (1, cancel_handler, &cancel_args);
 #endif
-  if (pid == (pid_t) 0)
-    {
-      /* Child side.  */
-      const char *new_argv[4];
-      new_argv[0] = SHELL_NAME;
-      new_argv[1] = "-c";
-      new_argv[2] = line;
-      new_argv[3] = NULL;
-
-      /* Restore the signals.  */
-      (void) __sigaction (SIGINT, &intr, (struct sigaction *) NULL);
-      (void) __sigaction (SIGQUIT, &quit, (struct sigaction *) NULL);
-      (void) __sigprocmask (SIG_SETMASK, &omask, (sigset_t *) NULL);
-      INIT_LOCK ();
-
-      /* Exec the shell.  */
-      (void) __execve (SHELL_PATH, (char *const *) new_argv, __environ);
-      _exit (127);
-    }
-  else if (pid < (pid_t) 0)
-    /* The fork failed.  */
-    status = -1;
-  else
-    /* Parent side.  */
-    {
       /* Note the system() is a cancellation point.  But since we call
 	 waitpid() which itself is a cancellation point we do not
 	 have to do anything here.  */
       if (TEMP_FAILURE_RETRY (__waitpid (pid, &status, 0)) != pid)
 	status = -1;
-    }
-
-#ifdef CLEANUP_HANDLER
-  CLEANUP_RESET;
+#if defined(_LIBC_REENTRANT) && defined(SIGCANCEL)
+      __libc_cleanup_region_end (0);
 #endif
+    }
 
-  save = errno;
   DO_LOCK ();
-  if ((SUB_REF () == 0
-       && (__sigaction (SIGINT, &intr, (struct sigaction *) NULL)
-	   | __sigaction (SIGQUIT, &quit, (struct sigaction *) NULL)) != 0)
-      || __sigprocmask (SIG_SETMASK, &omask, (sigset_t *) NULL) != 0)
+  if (SUB_REF () == 0)
     {
-#ifndef _LIBC
-      /* glibc cannot be used on systems without waitpid.  */
-      if (errno == ENOSYS)
-	__set_errno (save);
-      else
-#endif
-	status = -1;
+      /* sigaction can not fail with SIGINT/SIGQUIT used with old
+	 disposition.  Same applies for sigprocmask.  */
+      __sigaction (SIGINT, &intr, NULL);
+      __sigaction (SIGQUIT, &quit, NULL);
+      __sigprocmask (SIG_SETMASK, &omask, NULL);
     }
   DO_UNLOCK ();
 
diff --git a/sysdeps/unix/sysv/linux/ia64/system.c b/sysdeps/unix/sysv/linux/ia64/system.c
deleted file mode 100644
index d09fefefe6..0000000000
--- a/sysdeps/unix/sysv/linux/ia64/system.c
+++ /dev/null
@@ -1,30 +0,0 @@ 
-/* Copyright (C) 2002-2018 Free Software Foundation, Inc.
-   This file is part of the GNU C Library.
-
-   The GNU C Library is free software; you can redistribute it and/or
-   modify it under the terms of the GNU Lesser General Public
-   License as published by the Free Software Foundation; either
-   version 2.1 of the License, or (at your option) any later version.
-
-   The GNU C Library is distributed in the hope that it will be useful,
-   but WITHOUT ANY WARRANTY; without even the implied warranty of
-   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
-   Lesser General Public License for more details.
-
-   You should have received a copy of the GNU Lesser General Public
-   License along with the GNU C Library; if not, see
-   <http://www.gnu.org/licenses/>.  */
-
-/* We have to and actually can handle cancelable system().  The big
-   problem: we have to kill the child process if necessary.  To do
-   this a cleanup handler has to be registered and is has to be able
-   to find the PID of the child.  The main problem is to reliable have
-   the PID when needed.  It is not necessary for the parent thread to
-   return.  It might still be in the kernel when the cancellation
-   request comes.  Therefore we have to use the clone() calls ability
-   to have the kernel write the PID into the user-level variable.  */
-#define FORK() \
-  INLINE_SYSCALL (clone2, 6, CLONE_PARENT_SETTID | SIGCHLD, NULL, 0, \
-		  &pid, NULL, NULL)
-
-#include <sysdeps/unix/sysv/linux/system.c>
diff --git a/sysdeps/unix/sysv/linux/not-errno.h b/sysdeps/unix/sysv/linux/not-errno.h
index 106ba5c72e..b2f72cfb3d 100644
--- a/sysdeps/unix/sysv/linux/not-errno.h
+++ b/sysdeps/unix/sysv/linux/not-errno.h
@@ -16,6 +16,9 @@ 
    License along with the GNU C Library; if not, see
    <http://www.gnu.org/licenses/>.  */
 
+#include <sysdep.h>
+#include <fcntl.h>
+
 /* This function is used on maybe_enable_malloc_check (elf/dl-tunables.c)
    and to avoid having to build/use multiple versions if stack protection
    in enabled it is defined as inline.  */
@@ -33,3 +36,14 @@  __access_noerrno (const char *pathname, int mode)
     return INTERNAL_SYSCALL_ERRNO (res, err);
   return 0;
 }
+
+static inline int
+__kill_noerrno (pid_t pid, int sig)
+{
+  int res;
+  INTERNAL_SYSCALL_DECL (err);
+  res = INTERNAL_SYSCALL_CALL (kill, err, pid, sig);
+  if (INTERNAL_SYSCALL_ERROR_P (res, err))
+    return INTERNAL_SYSCALL_ERRNO (res, err);
+  return 0;
+}
diff --git a/sysdeps/unix/sysv/linux/s390/system.c b/sysdeps/unix/sysv/linux/s390/system.c
deleted file mode 100644
index d8ef461334..0000000000
--- a/sysdeps/unix/sysv/linux/s390/system.c
+++ /dev/null
@@ -1,29 +0,0 @@ 
-/* Copyright (C) 2003-2018 Free Software Foundation, Inc.
-   This file is part of the GNU C Library.
-
-   The GNU C Library is free software; you can redistribute it and/or
-   modify it under the terms of the GNU Lesser General Public
-   License as published by the Free Software Foundation; either
-   version 2.1 of the License, or (at your option) any later version.
-
-   The GNU C Library is distributed in the hope that it will be useful,
-   but WITHOUT ANY WARRANTY; without even the implied warranty of
-   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
-   Lesser General Public License for more details.
-
-   You should have received a copy of the GNU Lesser General Public
-   License along with the GNU C Library; if not, see
-   <http://www.gnu.org/licenses/>.  */
-
-/* We have to and actually can handle cancelable system().  The big
-   problem: we have to kill the child process if necessary.  To do
-   this a cleanup handler has to be registered and is has to be able
-   to find the PID of the child.  The main problem is to reliable have
-   the PID when needed.  It is not necessary for the parent thread to
-   return.  It might still be in the kernel when the cancellation
-   request comes.  Therefore we have to use the clone() calls ability
-   to have the kernel write the PID into the user-level variable.  */
-#define FORK() \
-  INLINE_SYSCALL (clone, 3, 0, CLONE_PARENT_SETTID | SIGCHLD, &pid)
-
-#include "../system.c"
diff --git a/sysdeps/unix/sysv/linux/sparc/system.c b/sysdeps/unix/sysv/linux/sparc/system.c
deleted file mode 100644
index 1f65c83399..0000000000
--- a/sysdeps/unix/sysv/linux/sparc/system.c
+++ /dev/null
@@ -1,29 +0,0 @@ 
-/* Copyright (C) 2003-2018 Free Software Foundation, Inc.
-   This file is part of the GNU C Library.
-
-   The GNU C Library is free software; you can redistribute it and/or
-   modify it under the terms of the GNU Lesser General Public
-   License as published by the Free Software Foundation; either
-   version 2.1 of the License, or (at your option) any later version.
-
-   The GNU C Library is distributed in the hope that it will be useful,
-   but WITHOUT ANY WARRANTY; without even the implied warranty of
-   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
-   Lesser General Public License for more details.
-
-   You should have received a copy of the GNU Lesser General Public
-   License along with the GNU C Library; if not, see
-   <http://www.gnu.org/licenses/>.  */
-
-/* We have to and actually can handle cancelable system().  The big
-   problem: we have to kill the child process if necessary.  To do
-   this a cleanup handler has to be registered and is has to be able
-   to find the PID of the child.  The main problem is to reliable have
-   the PID when needed.  It is not necessary for the parent thread to
-   return.  It might still be in the kernel when the cancellation
-   request comes.  Therefore we have to use the clone() calls ability
-   to have the kernel write the PID into the user-level variable.  */
-#define FORK() \
-  INLINE_CLONE_SYSCALL (CLONE_PARENT_SETTID | SIGCHLD, 0, &pid, NULL, NULL)
-
-#include "../system.c"
diff --git a/sysdeps/unix/sysv/linux/spawni.c b/sysdeps/unix/sysv/linux/spawni.c
index 9f3a137b5c..dbb6cdd5f0 100644
--- a/sysdeps/unix/sysv/linux/spawni.c
+++ b/sysdeps/unix/sysv/linux/spawni.c
@@ -138,11 +138,11 @@  __spawni_child (void *arguments)
   for (int sig = 1; sig < _NSIG; ++sig)
     {
       if ((attr->__flags & POSIX_SPAWN_SETSIGDEF)
-	  && sigismember (&attr->__sd, sig))
+	  && __sigismember (&attr->__sd, sig))
 	{
 	  sa.sa_handler = SIG_DFL;
 	}
-      else if (sigismember (&hset, sig))
+      else if (__sigismember (&hset, sig))
 	{
 	  if (__is_internal_signal (sig))
 	    sa.sa_handler = SIG_IGN;
diff --git a/sysdeps/unix/sysv/linux/system.c b/sysdeps/unix/sysv/linux/system.c
deleted file mode 100644
index 7cc68a1528..0000000000
--- a/sysdeps/unix/sysv/linux/system.c
+++ /dev/null
@@ -1,76 +0,0 @@ 
-/* Copyright (C) 2002-2018 Free Software Foundation, Inc.
-   This file is part of the GNU C Library.
-
-   The GNU C Library is free software; you can redistribute it and/or
-   modify it under the terms of the GNU Lesser General Public
-   License as published by the Free Software Foundation; either
-   version 2.1 of the License, or (at your option) any later version.
-
-   The GNU C Library is distributed in the hope that it will be useful,
-   but WITHOUT ANY WARRANTY; without even the implied warranty of
-   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
-   Lesser General Public License for more details.
-
-   You should have received a copy of the GNU Lesser General Public
-   License along with the GNU C Library; if not, see
-   <http://www.gnu.org/licenses/>.  */
-
-#include <sched.h>
-#include <signal.h>
-#include <string.h>	/* For the real memset prototype.  */
-#include <sysdep.h>
-#include <unistd.h>
-#include <sys/wait.h>
-#include <libc-lock.h>
-
-/* We have to and actually can handle cancelable system().  The big
-   problem: we have to kill the child process if necessary.  To do
-   this a cleanup handler has to be registered and is has to be able
-   to find the PID of the child.  The main problem is to reliable have
-   the PID when needed.  It is not necessary for the parent thread to
-   return.  It might still be in the kernel when the cancellation
-   request comes.  Therefore we have to use the clone() calls ability
-   to have the kernel write the PID into the user-level variable.  */
-#ifndef FORK
-# define FORK() \
-  INLINE_SYSCALL (clone, 3, CLONE_PARENT_SETTID | SIGCHLD, 0, &pid)
-#endif
-
-#ifdef _LIBC_REENTRANT
-static void cancel_handler (void *arg);
-
-# define CLEANUP_HANDLER \
-  __libc_cleanup_region_start (1, cancel_handler, &pid)
-
-# define CLEANUP_RESET \
-  __libc_cleanup_region_end (0)
-#endif
-
-
-/* Linux has waitpid(), so override the generic unix version.  */
-#include <sysdeps/posix/system.c>
-
-
-#ifdef _LIBC_REENTRANT
-/* The cancellation handler.  */
-static void
-cancel_handler (void *arg)
-{
-  pid_t child = *(pid_t *) arg;
-
-  INTERNAL_SYSCALL_DECL (err);
-  INTERNAL_SYSCALL (kill, err, 2, child, SIGKILL);
-
-  TEMP_FAILURE_RETRY (__waitpid (child, NULL, 0));
-
-  DO_LOCK ();
-
-  if (SUB_REF () == 0)
-    {
-      (void) __sigaction (SIGQUIT, &quit, (struct sigaction *) NULL);
-      (void) __sigaction (SIGINT, &intr, (struct sigaction *) NULL);
-    }
-
-  DO_UNLOCK ();
-}
-#endif