[v7,7/8] posix: Add PIDFDFORK_NOSIGCHLD for pidfd_fork

Message ID 20230803163558.991832-8-adhemerval.zanella@linaro.org
State Superseded
Headers
Series Add pidfd and cgroupv2 support for process creation |

Checks

Context Check Description
redhat-pt-bot/TryBot-apply_patch success Patch applied to master at the time it was sent
linaro-tcwg-bot/tcwg_glibc_check--master-aarch64 fail Testing failed
linaro-tcwg-bot/tcwg_glibc_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_glibc_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_glibc_check--master-arm fail Testing failed

Commit Message

Adhemerval Zanella Netto Aug. 3, 2023, 4:35 p.m. UTC
  It clones the process without setting SIGCHLD as the termination
signal.  When using this flag, the parent process must specify the
__WALL or __WCLONE Linux specific options when waiting for the child
with wait or waitid.

Checked on x86_64-linux-gnu and i686-linux-gnu.
---
 include/clone_internal.h                 |  3 +-
 manual/process.texi                      |  6 ++++
 sysdeps/nptl/_Fork.c                     |  2 +-
 sysdeps/unix/sysv/linux/arch-fork.h      |  2 +-
 sysdeps/unix/sysv/linux/clone-internal.c | 10 ++++--
 sysdeps/unix/sysv/linux/pidfd_fork.c     | 13 +++----
 sysdeps/unix/sysv/linux/sys/pidfd.h      |  2 ++
 sysdeps/unix/sysv/linux/tst-pidfd_fork.c | 45 ++++++++++++++++++++++--
 8 files changed, 69 insertions(+), 14 deletions(-)
  

Patch

diff --git a/include/clone_internal.h b/include/clone_internal.h
index d9b5509f78..4ec0c9198f 100644
--- a/include/clone_internal.h
+++ b/include/clone_internal.h
@@ -48,7 +48,8 @@  extern int __clone_internal_fallback (struct clone_args *__cl_args,
 
    It does not provide CLONE_INTO_CGROUP/CGROUP fallback if clone3 is not
    supported, in this case the function returns -1/ENOTSUP.  */
-extern int __clone_fork (uint64_t __extra_flags, void *__pidfd, int __cgroup)
+extern int __clone_fork (uint64_t __extra_flags, void *__pidfd, int __cgroup,
+			 bool nosigchld)
      attribute_hidden;
 
 /* Return whether the kernel supports pid file descriptor, including clone
diff --git a/manual/process.texi b/manual/process.texi
index a656df425b..c60701aeb8 100644
--- a/manual/process.texi
+++ b/manual/process.texi
@@ -390,6 +390,12 @@  following flags:
 Acts as @code{_Fork}, where it does not invoke any callbacks registered with
 @code{pthread_atfork}, nor does it reset internal state or locks (such as the
 @code{malloc} locks).
+
+@item PIDFDFORK_NOSIGCHLD
+Do not send a @code{SIGCHLD} termination signal when child terminates.
+@strong{NB:} When using this flag, the parent process must specify the
+@code{__WALL} or @code{__WCLONE} options when waiting for the child with
+@code{wait} or @code{waitid}.
 @end table
 @end deftypefun
 
diff --git a/sysdeps/nptl/_Fork.c b/sysdeps/nptl/_Fork.c
index aa99e05b5b..397f059fb0 100644
--- a/sysdeps/nptl/_Fork.c
+++ b/sysdeps/nptl/_Fork.c
@@ -22,7 +22,7 @@ 
 pid_t
 _Fork (void)
 {
-  pid_t pid = arch_fork (0, NULL, &THREAD_SELF->tid);
+  pid_t pid = arch_fork (SIGCHLD, NULL, &THREAD_SELF->tid);
   if (pid == 0)
     {
       struct pthread *self = THREAD_SELF;
diff --git a/sysdeps/unix/sysv/linux/arch-fork.h b/sysdeps/unix/sysv/linux/arch-fork.h
index 9e8a449e2c..f978d4c4f4 100644
--- a/sysdeps/unix/sysv/linux/arch-fork.h
+++ b/sysdeps/unix/sysv/linux/arch-fork.h
@@ -35,7 +35,7 @@  static inline pid_t
 arch_fork (int flags, void *ptid, void *ctid)
 {
   long int ret;
-  flags |= CLONE_CHILD_SETTID | CLONE_CHILD_CLEARTID | SIGCHLD;
+  flags |= CLONE_CHILD_SETTID | CLONE_CHILD_CLEARTID;
 #ifdef __ASSUME_CLONE_BACKWARDS
 # ifdef INLINE_CLONE_SYSCALL
   ret = INLINE_CLONE_SYSCALL (flags, 0, ptid, 0, ctid);
diff --git a/sysdeps/unix/sysv/linux/clone-internal.c b/sysdeps/unix/sysv/linux/clone-internal.c
index d212f2591f..615e79a510 100644
--- a/sysdeps/unix/sysv/linux/clone-internal.c
+++ b/sysdeps/unix/sysv/linux/clone-internal.c
@@ -125,7 +125,7 @@  __clone_internal (struct clone_args *cl_args,
 libc_hidden_def (__clone_internal)
 
 int
-__clone_fork (uint64_t extra_flags, void *pidfd, int cgroup)
+__clone_fork (uint64_t extra_flags, void *pidfd, int cgroup, bool nosigchld)
 {
 #ifdef __NR_clone3
   struct clone_args clone_args =
@@ -133,7 +133,7 @@  __clone_fork (uint64_t extra_flags, void *pidfd, int cgroup)
       .flags = extra_flags
 	       | CLONE_CHILD_SETTID
 	       | CLONE_CHILD_CLEARTID,
-      .exit_signal = SIGCHLD,
+      .exit_signal = nosigchld ? 0 : SIGCHLD,
       .cgroup = cgroup,
       .child_tid = (uintptr_t) &THREAD_SELF->tid,
       .pidfd = (uintptr_t) pidfd,
@@ -161,7 +161,11 @@  __clone_fork (uint64_t extra_flags, void *pidfd, int cgroup)
   bool use_pidfd = pidfd != NULL;
 
   if (!set_cgroup)
-    pid = arch_fork (use_pidfd ? CLONE_PIDFD : 0, pidfd, &THREAD_SELF->tid);
+    {
+      int extra_flags = use_pidfd ? CLONE_PIDFD : 0
+			| (nosigchld ? 0 : SIGCHLD);
+      pid = arch_fork (extra_flags, pidfd, &THREAD_SELF->tid);
+    }
   else
     {
       /* No fallback for POSIX_SPAWN_SETCGROUP if clone3 is not supported.  */
diff --git a/sysdeps/unix/sysv/linux/pidfd_fork.c b/sysdeps/unix/sysv/linux/pidfd_fork.c
index 983f8ade98..f3b6b74375 100644
--- a/sysdeps/unix/sysv/linux/pidfd_fork.c
+++ b/sysdeps/unix/sysv/linux/pidfd_fork.c
@@ -22,7 +22,7 @@ 
 #include <sys/pidfd.h>
 
 static pid_t
-forkfd (int *pidfd, int cgroup)
+forkfd (int *pidfd, int cgroup, bool nosigchld)
 {
   bool use_pidfd = pidfd != NULL;
   bool set_cgroup = cgroup != -1;
@@ -30,8 +30,7 @@  forkfd (int *pidfd, int cgroup)
   uint64_t extra_flags = (use_pidfd ? CLONE_PIDFD : 0)
 			 | (set_cgroup ? CLONE_INTO_CGROUP : 0);
   pid_t pid = __clone_fork (extra_flags, use_pidfd ? pidfd : NULL,
-			    set_cgroup ? cgroup: 0);
-
+			    set_cgroup ? cgroup: 0, nosigchld);
   if (pid == 0)
     {
       struct pthread *self = THREAD_SELF;
@@ -54,9 +53,11 @@  pidfd_fork (int *pidfd, int cgroup, unsigned int flags)
   if (!__clone_pidfd_supported ())
     return INLINE_SYSCALL_ERROR_RETURN_VALUE (ENOSYS);
 
-  if (flags & ~(PIDFDFORK_ASYNCSAFE))
+  if (flags & ~(PIDFDFORK_ASYNCSAFE | PIDFDFORK_NOSIGCHLD))
     return INLINE_SYSCALL_ERROR_RETURN_VALUE (EINVAL);
 
+  bool nosigchld = flags & PIDFDFORK_NOSIGCHLD;
+
   pid_t pid;
   if (!(flags & PIDFDFORK_ASYNCSAFE))
     {
@@ -67,7 +68,7 @@  pidfd_fork (int *pidfd, int cgroup, unsigned int flags)
       struct nss_database_data nss_database_data;
 
       state.lastrun = __fork_pre (multiple_threads, &nss_database_data);
-      state.pid = forkfd (pidfd, cgroup);
+      state.pid = forkfd (pidfd, cgroup, nosigchld);
       /* It follow the usual fork semantic, where a positive or negative
 	 value is returned to parent, and 0 for the child.  */
       __fork_post (&state, &nss_database_data);
@@ -75,7 +76,7 @@  pidfd_fork (int *pidfd, int cgroup, unsigned int flags)
       pid = state.pid;
     }
   else
-    pid = forkfd (pidfd, cgroup);
+    pid = forkfd (pidfd, cgroup, nosigchld);
 
   return pid;
 }
diff --git a/sysdeps/unix/sysv/linux/sys/pidfd.h b/sysdeps/unix/sysv/linux/sys/pidfd.h
index 3e6d009ce7..87095212a7 100644
--- a/sysdeps/unix/sysv/linux/sys/pidfd.h
+++ b/sysdeps/unix/sysv/linux/sys/pidfd.h
@@ -49,6 +49,8 @@  extern int pidfd_send_signal (int __pidfd, int __sig, siginfo_t *__info,
 
 /* Do not issue the pthread_atfork on pidfd_fork.  */
 #define PIDFDFORK_ASYNCSAFE (1U << 1)
+/* Do not send a SIGCHLD termination signal.  */
+#define PIDFDFORK_NOSIGCHLD (1U << 2)
 
 /* Clone the calling process, creating an exact copy and return a file
    descriptor that can be used along other pidfd functions.
diff --git a/sysdeps/unix/sysv/linux/tst-pidfd_fork.c b/sysdeps/unix/sysv/linux/tst-pidfd_fork.c
index 3e09c55d54..ee3a72ba5d 100644
--- a/sysdeps/unix/sysv/linux/tst-pidfd_fork.c
+++ b/sysdeps/unix/sysv/linux/tst-pidfd_fork.c
@@ -24,6 +24,7 @@ 
 #include <support/check.h>
 #include <support/temp_file.h>
 #include <support/xunistd.h>
+#include <support/xsignal.h>
 #include <sys/pidfd.h>
 #include <sys/wait.h>
 
@@ -33,6 +34,14 @@  static bool atfork_prepare_var;
 static bool atfork_parent_var;
 static bool atfork_child_var;
 
+static sig_atomic_t sigchld_called;
+
+static void
+sigchld_handler (int sig)
+{
+  sigchld_called = 1;
+}
+
 static void
 atfork_prepare (void)
 {
@@ -69,6 +78,9 @@  singlethread_test (unsigned int flags, bool wait_with_pid)
   off_t off = xlseek (tempfd, 0, SEEK_CUR);
   TEST_COMPARE (off, testdatalen1);
 
+  bool check_nosigchld = flags & PIDFDFORK_NOSIGCHLD;
+  sigchld_called = 0;
+
   int pidfd;
   pid_t pid = pidfd_fork (&pidfd, -1, flags);
   TEST_VERIFY_EXIT (pid != -1);
@@ -100,13 +112,18 @@  singlethread_test (unsigned int flags, bool wait_with_pid)
 
   {
     siginfo_t sinfo;
+    int options = WEXITED | (check_nosigchld ? __WCLONE : 0);
     if (wait_with_pid)
-      TEST_COMPARE (waitid (P_PID, pid, &sinfo, WEXITED), 0);
+      TEST_COMPARE (waitid (P_PID, pid, &sinfo, options), 0);
     else
-      TEST_COMPARE (waitid (P_PIDFD, pidfd, &sinfo, WEXITED), 0);
+      TEST_COMPARE (waitid (P_PIDFD, pidfd, &sinfo, options), 0);
     TEST_COMPARE (sinfo.si_signo, SIGCHLD);
     TEST_COMPARE (sinfo.si_code, CLD_EXITED);
     TEST_COMPARE (sinfo.si_status, 0);
+
+    /* If PIDFDFORK_NOSIGCHLD is specified no SIGCHLD should be sent by the
+       kernel.  */
+    TEST_COMPARE (sigchld_called, check_nosigchld ? 0 : 1);
   }
 
   TEST_COMPARE (xlseek (tempfd, 0, SEEK_CUR), testdatalen2);
@@ -140,6 +157,14 @@  do_test (void)
     TEST_COMPARE (sinfo.si_status, 0);
   }
 
+  {
+    struct sigaction sa;
+    sa.sa_handler = sigchld_handler;
+    sa.sa_flags = 0;
+    sigemptyset (&sa.sa_mask);
+    xsigaction (SIGCHLD, &sa, NULL);
+  }
+
   pthread_atfork (atfork_prepare, atfork_parent, atfork_child);
 
   /* With default flags, pidfd_fork acts as fork and run the pthread_atfork
@@ -161,6 +186,14 @@  do_test (void)
     TEST_VERIFY (!atfork_child_var);
   }
 
+  {
+    atfork_prepare_var = atfork_parent_var = atfork_child_var = false;
+    singlethread_test (PIDFDFORK_NOSIGCHLD, false);
+    TEST_VERIFY (atfork_prepare_var);
+    TEST_VERIFY (atfork_parent_var);
+    TEST_VERIFY (!atfork_child_var);
+  }
+
   /* With PIDFDFORK_ASYNCSAFE, pidfd_fork acts as _Fork.  */
   {
     atfork_prepare_var = atfork_parent_var = atfork_child_var = false;
@@ -180,6 +213,14 @@  do_test (void)
     TEST_VERIFY (!atfork_child_var);
   }
 
+  {
+    atfork_prepare_var = atfork_parent_var = atfork_child_var = false;
+    singlethread_test (PIDFDFORK_NOSIGCHLD | PIDFDFORK_ASYNCSAFE, true);
+    TEST_VERIFY (!atfork_prepare_var);
+    TEST_VERIFY (!atfork_parent_var);
+    TEST_VERIFY (!atfork_child_var);
+  }
+
   return 0;
 }