CLONE_FD: Task exit notification via file descriptor
v2 cover CLONE_FD: Task exit notification via file descriptor
This patch series introduces a new clone flag, CLONE_FD, which lets the caller receive child process exit notification via a file descriptor rather than SIGCHLD. CLONE_FD makes it possible for libraries to safely launch and manage child processes on behalf of their caller, *without* taking over process-wide SIGCHLD handling (either via signal handler or signalfd).
Note that signalfd for SIGCHLD does not suffice here, because that still receives notification for all child processes, and interferes with process-wide signal handling.
The CLONE_FD file descriptor uniquely identifies a process on the system in a race-free way, by holding a reference to the task_struct. In the future, we may introduce APIs that support using process file descriptors instead of PIDs.
This patch series also introduces a clone flag CLONE_AUTOREAP, which causes the kernel to automatically reap the child process when it exits, just as it does for processes using SIGCHLD when the parent has SIGCHLD ignored or marked as SA_NOCLDSTOP.
Taken together, a library can launch a process with CLONE_FD, CLONE_AUTOREAP, and no exit signal, and completely avoid affecting either process-wide signal handling or an existing child wait loop.
Introducing CLONE_FD and CLONE_AUTOREAP required two additional bits of yak
shaving: Since clone has no more usable flags (with the three currently unused flags unusable because old kernels ignore them without EINVAL), also introduce a new clone4 system call with more flag bits and an extensible argument structure. And since the magic pt_regs-based syscall argument processing for clone's tls argument would otherwise prevent introducing a sane clone4 system call, fix that too.
I tested the CLONE_SETTLS changes with a thread-local storage test program (two threads independently reading and writing a __thread variable), on both 32-bit and 64-bit, and I observed no issues there.
I tested clone4 and the new flags with several additional test programs, launching either a process or thread (in the former case using syscall(), in the latter case by calling clone4 via assembly and returning to C), sleeping in parent and child to test the case of either exiting first, and then printing the received clone4_info structure.
Changes in v2:
- Split out autoreaping into a separate CLONE_AUTOREAP. CLONE_FD no longer implies autoreaping and no exit signal, and CLONE_AUTOREAP does not affect ptracers or signal handling. Thanks to Oleg Nesterov for careful investigation and discussion on v1.
- Accept O_CLOEXEC and O_NONBLOCK via a clonefd_flags parameter in clone4_args. Stop overloading the low byte of the main clone flags, since CLONE_FD now works with a non-zero signal.
- Return the file descriptor via an out parameter in clone4_args.
- Drop patch to export alloc_fd; CLONE_FD now uses the next available file descriptor, even if that's 0-2, since clone4 no longer needs to avoid ambiguity with the 0 return indicating the child process.
- Make poll on a CLONE_FD for an exited task also return POLLHUP, for compatibility with FreeBSD's pdfork. Thanks to David Drysdale for calling attention to pdfork.
- Fix typo in squelch_clone_flags.
- Pass arguments to _do_fork and copy_process as a structure.
- Construct the 64-bit flags in a separate variable, rather than inline in the call to do_fork.
- Fix error return for copy_from_user faults.
- Add the new syscall to asm-generic.
- Add ack from Andy Lutomirski to patches 1 and 2.
I've included the manpages patch at the end of this series. (Note that the manpage documents the behavior of the future glibc wrapper as well as the raw syscall.) Here's a formatted plain-text version of the manpage for reference:
CLONE4(2) Linux Programmer's Manual CLONE4(2)
NAME
clone4 - create a child process
SYNOPSIS
/* Prototype for the glibc wrapper function */
#define _GNU_SOURCE
#include <sched.h>
int clone4(uint64_t flags,
size_t args_size,
struct clone4_args *args,
int (*fn)(void *), void *arg);
/* Prototype for the raw system call */
int clone4(unsigned flags_high, unsigned flags_low,
unsigned long args_size,
struct clone4_args *args);
struct clone4_args {
pid_t *ptid;
pid_t *ctid;
unsigned long stack_start;
unsigned long stack_size;
unsigned long tls;
int *clonefd;
unsigned clonefd_flags;
};DESCRIPTION
clone4() creates a new process, similar to clone(2) and fork(2). clone4() supports additional flags that clone(2) does not, and accepts arguments via an extensible structure.
args points to a clone4_args structure, and args_size must contain the
size of that structure, as understood by the caller. If the caller
passes a shorter structure than the kernel expects, the remaining fields will default to 0. If the caller passes a larger structure than the kernel expects (such as one from a newer kernel), clone4() will return EINVAL. The clone4_args structure may gain additional fields at the end in the future, and callers must only pass a size that encom‐ passes the number of fields they understand. If the caller passes 0 for args_size, args is ignored and may be NULL.
In the clone4_args structure, ptid, ctid, stack_start, stack_size, and tls have the same semantics as they do with clone(2) and clone2(2).
In the glibc wrapper, fn and arg have the same semantics as they do with clone(2). As with clone(2), the underlying system call works more like fork(2), returning 0 in the child process; the glibc wrapper sim‐ plifies thread execution by calling fn(arg) and exiting the child when that function exits.
The 64-bit flags argument (split into the 32-bit flags_high and flags_low arguments in the kernel interface for portability across architectures) accepts all the same flags as clone(2), with the excep‐ tion of the obsolete CLONE_PID, CLONE_DETACHED, and CLONE_STOPPED. In addition, flags accepts the following flags:
CLONE_AUTOREAP
When the new process exits, immediately reap it, rather than keeping it around as a "zombie" until a call to waitpid(2) or similar. Without this flag, the kernel will automatically reap a process if its exit signal is set to SIGCHLD, and if the par‐ ent process has SIGCHLD set to SIG_IGN or has a SIGCHLD handler installed with SA_NOCLDWAIT (see sigaction(2)). CLONE_AUTOREAP allows the calling process to enable automatic reaping with an exit signal other than SIGCHLD (including 0 to disable the exit signal), and does not depend on the configuration of process- wide signal handling.
CLONE_FD
Return a file descriptor associated with the new process, stor‐
ing it in location clonefd in the parent's address space. When
the new process exits, the file descriptor will become available for reading.
Unlike using signalfd(2) for the SIGCHLD signal, the file descriptor returned by clone4() with the CLONE_FD flag works even with SIGCHLD unblocked in one or more threads of the parent process, allowing the process to have different handlers for different child processes, such as those created by a library, without introducing race conditions around process-wide signal handling.
clonefd_flags may contain the following additional flags for use with CLONE_FD:
O_CLOEXEC
Set the close-on-exec flag on the new file descriptor. See the description of the O_CLOEXEC flag in open(2) for reasons why this may be useful.
O_NONBLOCK
Set the O_NONBLOCK flag on the new file descriptor. Using this flag saves extra calls to fcntl(2) to achieve the same result.
The returned file descriptor supports the following operations:
read(2) (and similar)
When the new process exits, reading from the file descriptor produces a single clonefd_info structure:
struct clonefd_info {
uint32_t code; /* Signal code */
uint32_t status; /* Exit status or signal */ uint64_t utime; /* User CPU time */ uint64_t stime; /* System CPU time */
};
If the new process has not yet exited, read(2) either blocks until it does, or fails with the error EAGAIN if the file descriptor has O_NONBLOCK set.
Future kernels may extend clonefd_info by appending addi‐ tional fields to the end. Callers should read as many bytes as they understand; unread data will be discarded, and subsequent reads after the first will return 0 to indicate end-of-file. Callers requesting more bytes than the kernel provides (such as callers expecting a newer clonefd_info structure) will receive a shorter structure from older kernels.
poll(2), select(2), epoll(7) (and similar)
The file descriptor is readable (the select(2) readfds argument; the poll(2) POLLIN flag) if the new process has exited.
close(2)
When the file descriptor is no longer required it should be closed.
C library/kernel ABI differences
As with clone(2), the raw clone4() system call corresponds more closely to fork(2) in that execution in the child continues from the point of the call.
Unlike clone(2), the raw system call interface for clone4() accepts arguments in the same order on all architectures.
The raw system call accepts flags as two 32-bit arguments, flags_high and flags_low, to simplify portability across 32-bit and 64-bit archi‐ tectures and calling conventions. The glibc wrapper accepts flags as a single 64-bit argument for convenience.
RETURN VALUE
For the glibc wrapper, on success, clone4() returns the new process ID to the calling process, and the new process begins running at the spec‐ ified function.
For the raw syscall, on success, clone4() returns the new process ID to the calling process, and returns 0 in the new process.
On failure, clone4() returns -1 and sets errno accordingly.
ERRORS
clone4() can return any error from clone(2), as well as the following additional errors:
EFAULT args is outside your accessible address space.
EINVAL flags contained an unknown flag.
EINVAL flags included CLONE_FD and clonefd_flags contained an unknown
flag.
EINVAL flags included CLONE_FD, but the kernel configuration does not
have the CONFIG_CLONEFD option enabled.
EMFILE flags included CLONE_FD, but the new file descriptor would
exceed the process limit on open file descriptors.
ENFILE flags included CLONE_FD, but the new file descriptor would
exceed the system-wide limit on open file descriptors.
ENODEV flags included CLONE_FD, but clone4() could not mount the
(internal) anonymous inode device.
CONFORMING TO
clone4() is Linux-specific and should not be used in programs intended to be portable.
SEE ALSO
clone(2), epoll(7), poll(2), pthreads(7), read(2), select(2)
Linux 2015-03-14 CLONE4(2)
Josh Triplett and Thiago Macieira (7):
clone: Support passing tls argument via C rather than pt_regs magic
x86: Opt into HAVE_COPY_THREAD_TLS, for both 32-bit and 64-bit Introduce a new clone4 syscall with more flag bits and extensible arguments kernel/fork.c: Pass arguments to _do_fork and copy_process using clone4_args
clone4: Add a CLONE_AUTOREAP flag to automatically reap the child process
signal: Factor out a helper function to process task_struct exit_code
clone4: Add a CLONE_FD flag to get task exit notification via fd
arch/Kconfig | 7 ++ arch/x86/Kconfig | 1 + arch/x86/ia32/ia32entry.S | 3 +- arch/x86/kernel/entry_64.S | 1 + arch/x86/kernel/process_32.c | 6 +- arch/x86/kernel/process_64.c | 8 +-- arch/x86/syscalls/syscall_32.tbl | 1 + arch/x86/syscalls/syscall_64.tbl | 2 + include/linux/compat.h | 14 ++++ include/linux/sched.h | 22 ++++++ include/linux/syscalls.h | 6 +- include/uapi/asm-generic/unistd.h | 4 +- include/uapi/linux/sched.h | 55 ++++++++++++++- init/Kconfig | 21 ++++++ kernel/Makefile | 1 + kernel/clonefd.c | 121 ++++++++++++++++++++++++++++++++ kernel/clonefd.h | 32 +++++++++ kernel/exit.c | 4 ++ kernel/fork.c | 142 ++++++++++++++++++++++++++++++-------- kernel/signal.c | 26 ++++--- kernel/sys_ni.c | 1 + 21 files changed, 426 insertions(+), 52 deletions(-) create mode 100644 kernel/clonefd.c create mode 100644 kernel/clonefd.h
Signature · 1 line
2.1.4
PATCH v2 1/7 clone: Support passing tls argument via C rather than pt_regs magic
clone with CLONE_SETTLS accepts an argument to set the thread-local storage area for the new thread. sys_clone declares an int argument tls_val in the appropriate point in the argument list (based on the various CLONE_BACKWARDS variants), but doesn't actually use or pass along that argument. Instead, sys_clone calls do_fork, which calls copy_process, which calls the arch-specific copy_thread, and copy_thread pulls the corresponding syscall argument out of the pt_regs captured at kernel entry (knowing what argument of clone that architecture passes tls in).
Apart from being awful and inscrutable, that also only works because only one code path into copy_thread can pass the CLONE_SETTLS flag, and that code path comes from sys_clone with its architecture-specific argument-passing order. This prevents introducing a new version of the clone system call without propagating the same architecture-specific position of the tls argument.
However, there's no reason to pull the argument out of pt_regs when sys_clone could just pass it down via C function call arguments.
Introduce a new CONFIG_HAVE_COPY_THREAD_TLS for architectures to opt into, and a new copy_thread_tls that accepts the tls parameter as an additional unsigned long (syscall-argument-sized) argument. Change sys_clone's tls argument to an unsigned long (which does not change the ABI), and pass that down to copy_thread_tls.
Architectures that don't opt into copy_thread_tls will continue to ignore the C argument to sys_clone in favor of the pt_regs captured at kernel entry, and thus will be unable to introduce new versions of the clone syscall.
--- arch/Kconfig | 7 ++++++ include/linux/sched.h | 14 ++++++++++++ include/linux/syscalls.h | 6 +++--- kernel/fork.c | 55 +++++++++++++++++++++++++++++++----------------- 4 files changed, 60 insertions(+), 22 deletions(-)
arch/Kconfig +7 -0
@@ -484,6 +484,13 @@ config HAVE_IRQ_EXIT_ON_IRQ_STACK This spares a stack switch and improves cache usage on softirq processing. +config HAVE_COPY_THREAD_TLS + bool + help + Architecture provides copy_thread_tls to accept tls argument via + normal C parameter passing, rather than extracting the syscall + argument from pt_regs. + # # ABI hall of shame #
include/linux/sched.h +14 -0
@@ -2479,8 +2479,22 @@ extern struct mm_struct *mm_access(struct task_struct *task, unsigned int mode);/* Remove the current tasks stale references to the old mm_struct */ extern void mm_release (struct task_struct *,struct mm_struct *); +#ifdef CONFIG_HAVE_COPY_THREAD_TLS +extern int copy_thread_tls (unsigned long ,unsigned long ,unsigned long , +struct task_struct *,unsigned long ); +#else extern int copy_thread (unsigned long ,unsigned long ,unsigned long ,struct task_struct *); + +/* Architectures that haven't opted into copy_thread_tls get the tls argument +* via pt_regs, so ignore the tls argument passed via C. */ +static inline int copy_thread_tls ( +unsigned long clone_flags,unsigned long sp,unsigned long arg, +struct task_struct *p,unsigned long tls) +{ +return copy_thread (clone_flags, sp, arg, p); +} +#endif extern void flush_thread (void );extern void exit_thread (void );
include/linux/syscalls.h +3 -3
@@ -827,15 +827,15 @@ asmlinkage long sys_syncfs(int fd);asmlinkage long sys_fork (void );asmlinkage long sys_vfork (void );#ifdef CONFIG_CLONE_BACKWARDS -asmlinkage long sys_clone (unsigned long ,unsigned long ,int __user *,int , +asmlinkage long sys_clone (unsigned long ,unsigned long ,int __user *,unsigned long ,int __user *);#else #ifdef CONFIG_CLONE_BACKWARDS3 asmlinkage long sys_clone (unsigned long ,unsigned long ,int ,int __user *, -int __user *,int ); +int __user *,unsigned long );#else asmlinkage long sys_clone (unsigned long ,unsigned long ,int __user *, -int __user *,int ); +int __user *,unsigned long );#endif #endif
kernel/fork.c +36 -19
130 lines not on this page · raw message
Signature · 1 line
2.1.4
PATCH v2 2/7 x86: Opt into HAVE_COPY_THREAD_TLS, for both 32-bit and 64-bit
In reply to Josh TriplettFor 32-bit userspace on a 64-bit kernel, this requires modifying stub32_clone to actually swap the appropriate arguments to match CONFIG_CLONE_BACKWARDS, rather than just leaving the C argument for tls broken.
--- arch/x86/Kconfig | 1 + arch/x86/ia32/ia32entry.S | 2 +- arch/x86/kernel/process_32.c | 6 +++--- arch/x86/kernel/process_64.c | 8 ++++---- 4 files changed, 9 insertions(+), 8 deletions(-)
arch/x86/Kconfig +1 -0
@@ -124,6 +124,7 @@ config X86 select MODULES_USE_ELF_REL if X86_32 select MODULES_USE_ELF_RELA if X86_64 select CLONE_BACKWARDS if X86_32 + select HAVE_COPY_THREAD_TLS select ARCH_USE_BUILTIN_BSWAP select ARCH_USE_QUEUE_RWLOCK select OLD_SIGSUSPEND3 if X86_32 || IA32_EMULATION
arch/x86/ia32/ia32entry.S +1 -1
@@ -487,7 +487,7 @@ GLOBAL(\label) ALIGN GLOBAL(stub32_clone) leaq sys_clone(%rip),%rax - mov %r8, %rcx + xchg %r8, %rcx jmp ia32_ptregs_common ALIGN
arch/x86/kernel/process_32.c +3 -3
@@ -129,8 +129,8 @@ void release_thread(struct task_struct *dead_task)release_vm86_irqs (dead_task); } -int copy_thread (unsigned long clone_flags,unsigned long sp, -unsigned long arg,struct task_struct *p) +int copy_thread_tls (unsigned long clone_flags,unsigned long sp, +unsigned long arg,struct task_struct *p,unsigned long tls) {struct pt_regs *childregs =task_pt_regs (p);struct task_struct *tsk;
@@ -185,7 +185,7 @@ int copy_thread(unsigned long clone_flags, unsigned long sp,*/ if (clone_flags &CLONE_SETTLS ) err =do_set_thread_area (p,-1 , - (struct user_desc __user *)childregs->si,0 ); + (struct user_desc __user *)tls,0 );if (err && p->thread.io_bitmap_ptr) {kfree (p->thread.io_bitmap_ptr);
arch/x86/kernel/process_64.c +4 -4
@@ -151,8 +151,8 @@ static inline u32 read_32bit_tls(struct task_struct *t, int tls)return get_desc_base (&t->thread.tls_array[tls]); } -int copy_thread (unsigned long clone_flags,unsigned long sp, -unsigned long arg,struct task_struct *p) +int copy_thread_tls (unsigned long clone_flags,unsigned long sp, +unsigned long arg,struct task_struct *p,unsigned long tls) {int err;struct pt_regs *childregs;
@@ -209,10 +209,10 @@ int copy_thread(unsigned long clone_flags, unsigned long sp,#ifdef CONFIG_IA32_EMULATION if (test_thread_flag (TIF_IA32 )) err =do_set_thread_area (p,-1 , - (struct user_desc __user *)childregs->si,0 ); + (struct user_desc __user *)tls,0 ); else#endif - err =do_arch_prctl (p,ARCH_SET_FS , childregs->r8); + err =do_arch_prctl (p,ARCH_SET_FS , tls);if (err) goto out; }
Signature · 1 line
2.1.4
PATCH v2 3/7 Introduce a new clone4 syscall with more flag bits and extensible arguments
In reply to Josh Triplettclone() has no more usable flags available. It has three now-unused flags (CLONE_PID, CLONE_DETACHED, and CLONE_STOPPED), but current kernels just ignore those flags without returning an error like EINVAL, so reusing those flags would not allow userspace to detect the availability of the new functionality.
Introduce a new system call, clone4, which accepts a second 32-bit flags field. clone4 also returns EINVAL for the currently unused flags in clone, allowing their reuse.
To process these new flags, change the flags argument of _do_fork to a u64. sys_clone and do_fork both still use "unsigned long" for flags as they did before, truncating it to 32-bit and masking out the obsolete flags to behave like clone currently does.
clone4 accepts its remaining arguments as a structure, and userspace passes in the size of that structure. clone4 has well-defined semantics that allow extending that structure in the future. New userspace passing in a larger structure than the kernel expects will receive EINVAL, and can use a smaller structure to work with old kernels. New kernels accept smaller argument structures passed by userspace, and any un-passed arguments default to 0.
clone4 handles arguments in the same order on all architectures, with no backwards variations; to do so, it depends on the new HAVE_COPY_THREAD_TLS.
The new system call currently accepts exactly the same flags as clone; future commits will introduce new flags for additional functionality.
--- arch/x86/ia32/ia32entry.S | 1 + arch/x86/kernel/entry_64.S | 1 + arch/x86/syscalls/syscall_32.tbl | 1 + arch/x86/syscalls/syscall_64.tbl | 2 ++ include/linux/compat.h | 12 +++++++++ include/uapi/asm-generic/unistd.h | 4 ++- include/uapi/linux/sched.h | 36 ++++++++++++++++++++++--- init/Kconfig | 10 +++++++ kernel/fork.c | 56 ++++++++++++++++++++++++++++++++++++--- kernel/sys_ni.c | 1 + 10 files changed, 116 insertions(+), 8 deletions(-)
arch/x86/ia32/ia32entry.S +1 -0
@@ -483,6 +483,7 @@ GLOBAL(\label) PTREGSCALL stub32_execveat, compat_sys_execveat PTREGSCALL stub32_fork, sys_fork PTREGSCALL stub32_vfork, sys_vfork + PTREGSCALL stub32_clone4, compat_sys_clone4 ALIGN GLOBAL(stub32_clone)
arch/x86/kernel/entry_64.S +1 -0
@@ -520,6 +520,7 @@ END(\label) FORK_LIKE clone FORK_LIKE fork FORK_LIKE vfork + FORK_LIKE clone4 FIXED_FRAME stub_iopl, sys_iopl ENTRY(stub_execve)
arch/x86/syscalls/syscall_32.tbl +1 -0
@@ -365,3 +365,4 @@ 356 i386 memfd_create sys_memfd_create 357 i386 bpf sys_bpf 358 i386 execveat sys_execveat stub32_execveat +359 i386 clone4 sys_clone4 stub32_clone4
arch/x86/syscalls/syscall_64.tbl +2 -0
@@ -329,6 +329,7 @@ 320 common kexec_file_load sys_kexec_file_load 321 common bpf sys_bpf 322 64 execveat stub_execveat +323 64 clone4 stub_clone4 # # x32-specific system call numbers start at 512 to avoid cache impact
@@ -368,3 +369,4 @@ 543 x32 io_setup compat_sys_io_setup 544 x32 io_submit compat_sys_io_submit 545 x32 execveat stub_x32_execveat +546 x32 clone4 stub32_clone4
include/linux/compat.h +12 -0
@@ -293,6 +293,14 @@ struct compat_old_sigaction { };#endif +struct compat_clone4_args { +compat_uptr_t ptid; +compat_uptr_t ctid; +compat_ulong_t stack_start; +compat_ulong_t stack_size; +compat_ulong_t tls; +}; +struct compat_statfs ;struct compat_statfs64 ;struct compat_old_linux_dirent ;
@@ -713,6 +721,10 @@ asmlinkage long compat_sys_sched_rr_get_interval(compat_pid_t pid,asmlinkage long compat_sys_fanotify_mark (int ,unsigned int ,__u32 ,__u32 ,int ,const char __user *); + +asmlinkage long compat_sys_clone4 (unsigned ,unsigned ,compat_ulong_t , +struct compat_clone4_args __user *); +#else #define is_compat_task () (0)
include/uapi/asm-generic/unistd.h +3 -1
@@ -709,9 +709,11 @@ __SYSCALL(__NR_memfd_create, sys_memfd_create) __SYSCALL(__NR_bpf, sys_bpf)#define __NR_execveat 281 __SC_COMP(__NR_execveat, sys_execveat, compat_sys_execveat) +#define __NR_clone4 282 +__SC_COMP(__NR_clone4, sys_clone4, compat_sys_clone4)#undef __NR_syscalls -#define __NR_syscalls 282 +#define __NR_syscalls 283/* * All syscalls below here should go away really,
include/uapi/linux/sched.h +33 -3
@@ -1,6 +1,8 @@#ifndef _UAPI_LINUX_SCHED_H#define _UAPI_LINUX_SCHED_H +#include <linux/types.h> +/* * cloning flags: */
@@ -18,11 +20,8 @@#define CLONE_SETTLS 0x00080000/* create a new TLS for the child */ #define CLONE_PARENT_SETTID 0x00100000/* set the TID in the parent */ #define CLONE_CHILD_CLEARTID 0x00200000/* clear the TID in the child */ -#define CLONE_DETACHED 0x00400000/* Unused, ignored */ #define CLONE_UNTRACED 0x00800000/* set if the tracing process can't force CLONE_PTRACE on this clone */ #define CLONE_CHILD_SETTID 0x01000000/* set the TID in the child */ -/* 0x02000000 was previously the unused CLONE_STOPPED (Start in stopped state) -and is now available for re-use. */ #define CLONE_NEWUTS 0x04000000/* New utsname namespace */ #define CLONE_NEWIPC 0x08000000/* New ipc namespace */ #define CLONE_NEWUSER 0x10000000/* New user namespace */
@@ -31,6 +30,37 @@#define CLONE_IO 0x80000000/* Clone io context */ /* +* Old flags, unused by current clone. clone does not return EINVAL for these +* flags, so they can't easily be reused. clone4 can use them. +*/ +#define CLONE_PID 0x00001000 +#define CLONE_DETACHED 0x00400000 +#define CLONE_STOPPED 0x02000000 + +#ifdef __KERNEL__ +/* +* Valid flags for clone and for clone4. Kept in this file next to the flag +* list above, but not exposed to userspace. +*/ +#define CLONE_VALID_FLAGS (0xffffffffULL & ~(CLONE_PID | CLONE_DETACHED | CLONE_STOPPED)) +#define CLONE4_VALID_FLAGS CLONE_VALID_FLAGS +#endif /* __KERNEL__ */ + +/* +* Structure passed to clone4 for additional arguments. Initialized to 0, +* then overwritten with arguments from userspace, so arguments not supplied by +* userspace will remain 0. New versions of the kernel may safely append new +* arguments to the end. +*/ +struct clone4_args { +__kernel_pid_t __user *ptid; +__kernel_pid_t __user *ctid; +__kernel_ulong_t stack_start; +__kernel_ulong_t stack_size; +__kernel_ulong_t tls; +}; + +/* * Scheduling policies */ #define SCHED_NORMAL 0
init/Kconfig +10 -0
@@ -1511,6 +1511,16 @@ config EVENTFD If unsure, say Y. +config CLONE4 + bool "Enable clone4() system call" if EXPERT + depends on HAVE_COPY_THREAD_TLS + default y + help + Enable the clone4() system call, which supports passing additional + flags. + + If unsure, say Y. + # syscall, maps, verifier config BPF_SYSCALL bool "Enable bpf() system call" if EXPERT
kernel/fork.c +52 -4
95 lines not on this page · raw message
kernel/sys_ni.c +1 -0
@@ -159,6 +159,7 @@ cond_syscall(sys_uselib);cond_syscall (sys_fadvise64);cond_syscall (sys_fadvise64_64);cond_syscall (sys_madvise); +cond_syscall (sys_clone4);/* arch-specific weak syscall entries */ cond_syscall (sys_pciconfig_read);
Signature · 1 line
2.1.4
Re: [PATCH v2 3/7] Introduce a new clone4 syscall with more flag bits and extensible arguments
Quoted 46 lines · Josh Triplett
arch/x86/ia32/ia32entry.S +1
-0@@ -483,6 +483,7 @@ GLOBAL(\label) PTREGSCALL stub32_execveat, compat_sys_execveat PTREGSCALL stub32_fork, sys_fork PTREGSCALL stub32_vfork, sys_vfork + PTREGSCALL stub32_clone4, compat_sys_clone4 ALIGN GLOBAL(stub32_clone)arch/x86/kernel/entry_64.S +1
-0@@ -520,6 +520,7 @@ END(\label) FORK_LIKE clone FORK_LIKE fork FORK_LIKE vfork + FORK_LIKE clone4 FIXED_FRAME stub_iopl, sys_iopl ENTRY(stub_execve)arch/x86/syscalls/syscall_32.tbl +1
-0@@ -365,3 +365,4 @@ 356 i386 memfd_create sys_memfd_create 357 i386 bpf sys_bpf 358 i386 execveat sys_execveat stub32_execveat +359 i386 clone4 sys_clone4 stub32_clone4arch/x86/syscalls/syscall_64.tbl +1
-0@@ -329,6 +329,7 @@ 320 common kexec_file_load sys_kexec_file_load 321 common bpf sys_bpf 322 64 execveat stub_execveat +323 64 clone4 stub_clone4 # # x32-specific system call numbers start at 512 to avoid cache impact
arch/x86/syscalls/syscall_64.tbl line 372
Quoted 4 lines
arch/x86/syscalls/syscall_64.tbl +1
-0@@ -368,3 +369,4 @@ 543 x32 io_setup compat_sys_io_setup 544 x32 io_submit compat_sys_io_submit 545 x32 execveat stub_x32_execveat +546 x32 clone4 stub32_clone4
Doesn't this need an x32 specific wrapper (to ensure the full set of registers are saved)?
Re: [PATCH v2 3/7] Introduce a new clone4 syscall with more flag bits and extensible arguments
Quoted 44 lines · David Drysdale
Quoted 43 lines · Josh Triplett
arch/x86/ia32/ia32entry.S +1
-0@@ -483,6 +483,7 @@ GLOBAL(\label) PTREGSCALL stub32_execveat, compat_sys_execveat PTREGSCALL stub32_fork, sys_fork PTREGSCALL stub32_vfork, sys_vfork + PTREGSCALL stub32_clone4, compat_sys_clone4 ALIGN GLOBAL(stub32_clone)arch/x86/kernel/entry_64.S +1
-0@@ -520,6 +520,7 @@ END(\label) FORK_LIKE clone FORK_LIKE fork FORK_LIKE vfork + FORK_LIKE clone4 FIXED_FRAME stub_iopl, sys_iopl ENTRY(stub_execve)arch/x86/syscalls/syscall_32.tbl +1
-0@@ -365,3 +365,4 @@ 356 i386 memfd_create sys_memfd_create 357 i386 bpf sys_bpf 358 i386 execveat sys_execveat stub32_execveat +359 i386 clone4 sys_clone4 stub32_clone4arch/x86/syscalls/syscall_64.tbl +1
-0@@ -329,6 +329,7 @@ 320 common kexec_file_load sys_kexec_file_load 321 common bpf sys_bpf 322 64 execveat stub_execveat +323 64 clone4 stub_clone4
Quoted 10 lines
arch/x86/syscalls/syscall_64.tbl line 372
Quoted 7 lines
arch/x86/syscalls/syscall_64.tbl +1
-0# # x32-specific system call numbers start at 512 to avoid cache impact@@ -368,3 +369,4 @@ 543 x32 io_setup compat_sys_io_setup 544 x32 io_submit compat_sys_io_submit 545 x32 execveat stub_x32_execveat +546 x32 clone4 stub32_clone4Doesn't this need an x32 specific wrapper (to ensure the full set of registers are saved)?
I'm not an x32 expert; I don't know how x32 interacts with pt_regs and compat syscalls. Could an x32 expert weigh in, please?
- Josh Triplett
Re: [PATCH v2 3/7] Introduce a new clone4 syscall with more flag bits and extensible arguments
Quoted 56 lines · On Mon, Mar 23, 2015 at 3:05 PM, <[email protected]> wrote:
Quoted 44 lines · David Drysdale
Quoted 43 lines · Josh Triplett
arch/x86/ia32/ia32entry.S +1
-0@@ -483,6 +483,7 @@ GLOBAL(\label) PTREGSCALL stub32_execveat, compat_sys_execveat PTREGSCALL stub32_fork, sys_fork PTREGSCALL stub32_vfork, sys_vfork + PTREGSCALL stub32_clone4, compat_sys_clone4 ALIGN GLOBAL(stub32_clone)arch/x86/kernel/entry_64.S +1
-0@@ -520,6 +520,7 @@ END(\label) FORK_LIKE clone FORK_LIKE fork FORK_LIKE vfork + FORK_LIKE clone4 FIXED_FRAME stub_iopl, sys_iopl ENTRY(stub_execve)arch/x86/syscalls/syscall_32.tbl +1
-0@@ -365,3 +365,4 @@ 356 i386 memfd_create sys_memfd_create 357 i386 bpf sys_bpf 358 i386 execveat sys_execveat stub32_execveat +359 i386 clone4 sys_clone4 stub32_clone4arch/x86/syscalls/syscall_64.tbl +1
-0@@ -329,6 +329,7 @@ 320 common kexec_file_load sys_kexec_file_load 321 common bpf sys_bpf 322 64 execveat stub_execveat +323 64 clone4 stub_clone4Quoted 10 lines
arch/x86/syscalls/syscall_64.tbl line 372
Quoted 7 lines
arch/x86/syscalls/syscall_64.tbl +1
-0# # x32-specific system call numbers start at 512 to avoid cache impact@@ -368,3 +369,4 @@ 543 x32 io_setup compat_sys_io_setup 544 x32 io_submit compat_sys_io_submit 545 x32 execveat stub_x32_execveat +546 x32 clone4 stub32_clone4Doesn't this need an x32 specific wrapper (to ensure the full set of registers are saved)?
Quoted 4 lines
I'm not an x32 expert; I don't know how x32 interacts with
pt_regsand compat syscalls. Could an x32 expert weigh in, please?- Josh Triplett
(In the absence of an x32 expert chiming in...)
As I understand it:
- stub32_clone4 expects 32-bit calling conventions and calls compat_sys_clone4
- stub_clone4 expects 64-bit calling conventions and calls sys_clone4
- stub_x32_clone4 would expect 64-bit calling conventions but call compat_sys_clone4.
Also, I have a suspicion that different field types in the [compat_]clone4_args structure may cause problems -- I *think* its (final) layout will be 4+4+4+4+4+4 on 32-bit, 8+8+8+8+8+4 on 64-bit, but 4+4+8+8+4+4 on x32.
Have you tried running a test with a userspace program compiled with -mx32?
PATCH v2 4/7 kernel/fork.c: Pass arguments to _do_fork and copy_process using clone4_args
In reply to Josh TriplettRather than continuing to add arguments to _do_fork and copy_process for future clone4 extensions, with corresponding churn in every caller, pass the arguments using the clone4_args structure instead. This allows clone4 to avoid unpacking the arguments, and allows other callers to use C99 structure initializers to only initialize the arguments they care about. Future extensions to clone4_args will thus not need to touch clone4, fork, vfork, or other callers of _do_fork.
--- kernel/fork.c | 77 +++++++++++++++++++++++++++++++---------------------------- 1 file changed, 41 insertions(+), 36 deletions(-)
kernel/fork.c +41 -36
@@ -1188,12 +1188,9 @@ init_task_pid(struct task_struct *task, enum pid_type type, struct pid *pid)* flags). The actual kick-off is left to the caller. */ static struct task_struct *copy_process (u64 clone_flags, -unsigned long stack_start, -unsigned long stack_size, -int __user *child_tidptr, +struct clone4_args *args,struct pid *pid, -int trace, -unsigned long tls) +int trace) {int retval;struct task_struct *p;
@@ -1405,7 +1402,7 @@ static struct task_struct *copy_process(u64 clone_flags, retval =copy_io (clone_flags, p);if (retval) goto bad_fork_cleanup_namespaces; - retval =copy_thread_tls (clone_flags, stack_start, stack_size, p, tls); + retval =copy_thread_tls (clone_flags, args->stack_start, args->stack_size, p, args->tls);if (retval) goto bad_fork_cleanup_io;
@@ -1416,11 +1413,11 @@ static struct task_struct *copy_process(u64 clone_flags, goto bad_fork_cleanup_io; } - p->set_child_tid = (clone_flags &CLONE_CHILD_SETTID ) ? child_tidptr :NULL ; + p->set_child_tid = (clone_flags &CLONE_CHILD_SETTID ) ? args->ctid :NULL ;/* * Clear TID on mm_release()? */ - p->clear_child_tid = (clone_flags &CLONE_CHILD_CLEARTID ) ? child_tidptr :NULL ; + p->clear_child_tid = (clone_flags &CLONE_CHILD_CLEARTID ) ? args->ctid :NULL ;#ifdef CONFIG_BLOCK p->plug =NULL ;#endif
@@ -1617,7 +1614,8 @@ static inline void init_idle_pids(struct pid_link *links)struct task_struct *fork_idle (int cpu) {struct task_struct *task; - task =copy_process (CLONE_VM ,0 ,0 ,NULL , &init_struct_pid,0 ,0 ); +struct clone4_args args = {}; + task =copy_process (CLONE_VM , &args, &init_struct_pid,0 );if (!IS_ERR (task)) {init_idle_pids (task->pids);init_idle (task, cpu);
@@ -1632,13 +1630,7 @@ struct task_struct *fork_idle(int cpu)* It copies the process, and if successful kick-starts * it and waits for it to finish using the VM if required. */ -static long _do_fork ( -u64 clone_flags, -unsigned long stack_start, -unsigned long stack_size, -int __user *parent_tidptr, -int __user *child_tidptr, -unsigned long tls) +static long _do_fork (u64 clone_flags,struct clone4_args *args) {struct task_struct *p;int trace =0 ;
@@ -1662,8 +1654,7 @@ static long _do_fork( trace =0 ; } - p =copy_process (clone_flags, stack_start, stack_size, - child_tidptr,NULL , trace, tls); + p =copy_process (clone_flags, args,NULL , trace);/* * Do this prior waking up the new thread - the thread pointer * might get invalid after that point, if the thread exits quickly.
@@ -1678,7 +1669,7 @@ static long _do_fork( nr =pid_vnr (pid);if (clone_flags &CLONE_PARENT_SETTID ) -put_user (nr, parent_tidptr); +put_user (nr, args->ptid);if (clone_flags &CLONE_VFORK ) { p->vfork_done = &vfork;
@@ -1722,9 +1713,13 @@ long do_fork(unsigned long clone_flags,int __user *parent_tidptr, int __user *child_tidptr) { - return _do_fork(squelch_clone_flags(clone_flags), -stack_start ,stack_size , - parent_tidptr, child_tidptr, 0); + struct clone4_args kargs = { + .ptid = parent_tidptr, + .ctid = child_tidptr, + .stack_start = stack_start, + .stack_start = stack_size, + }; +return _do_fork (squelch_clone_flags (clone_flags), &kargs); }#endif
@@ -1733,15 +1728,19 @@ long do_fork(unsigned long clone_flags,*/ pid_t kernel_thread (int (*fn)(void *),void *arg,unsigned long flags) { -return _do_fork (flags|CLONE_VM |CLONE_UNTRACED , (unsigned long )fn, - (unsigned long )arg,NULL ,NULL ,0 ); +struct clone4_args kargs = { + .stack_start = (unsigned long )fn, + .stack_size = (unsigned long )arg, + }; +return _do_fork (flags|CLONE_VM |CLONE_UNTRACED , &kargs); }#ifdef __ARCH_WANT_SYS_FORKSYSCALL_DEFINE0 (fork) {#ifdef CONFIG_MMU -return _do_fork (SIGCHLD ,0 ,0 ,NULL ,NULL ,0 ); +struct clone4_args kargs = {}; +return _do_fork (SIGCHLD , &kargs);#else /* can not support in nommu mode */ return -EINVAL ;
@@ -1752,8 +1751,8 @@ SYSCALL_DEFINE0(fork)#ifdef __ARCH_WANT_SYS_VFORKSYSCALL_DEFINE0 (vfork) { -return _do_fork (CLONE_VFORK |CLONE_VM |SIGCHLD ,0 , -0 ,NULL ,NULL ,0 ); +struct clone4_args kargs = {}; +return _do_fork (CLONE_VFORK |CLONE_VM |SIGCHLD , &kargs); }#endif
@@ -1781,8 +1780,13 @@ SYSCALL_DEFINE5(clone, unsigned long, clone_flags, unsigned long, newsp, unsigned long, tls) #endif { -return _do_fork (squelch_clone_flags (clone_flags), newsp,0 , - parent_tidptr, child_tidptr, tls); +struct clone4_args kargs = { + .ptid = parent_tidptr, + .ctid = child_tidptr, + .stack_start = newsp, + .tls = tls, + }; +return _do_fork (squelch_clone_flags (clone_flags), &kargs); }#endif
@@ -1796,8 +1800,7 @@ SYSCALL_DEFINE4(clone4, unsigned, flags_high, unsigned, flags_low,return -EINVAL ;if (args_size &©_from_user (&kargs, args, args_size))return -EFAULT ; -return _do_fork (flags, kargs.stack_start, kargs.stack_size, - kargs.ptid, kargs.ctid, kargs.tls); +return _do_fork (flags, &kargs); }#ifdef CONFIG_COMPAT
@@ -1807,15 +1810,17 @@ COMPAT_SYSCALL_DEFINE4(clone4, unsigned, flags_high, unsigned, flags_low, {u64 flags = (u64 )flags_high <<32 | flags_low;struct compat_clone4_args compat_kargs = {}; +struct clone4_args kargs = {};if (args_size >sizeof (compat_kargs))return -EINVAL ;if (args_size &©_from_user (&compat_kargs, args, args_size))return -EFAULT ; -return _do_fork (flags, compat_kargs.stack_start, - compat_kargs.stack_size, -compat_ptr (compat_kargs.ptid), -compat_ptr (compat_kargs.ctid), - compat_kargs.tls); + kargs.ptid =compat_ptr (compat_kargs.ptid); + kargs.ctid =compat_ptr (compat_kargs.ctid); + kargs.stack_start = compat_kargs.stack_start; + kargs.stack_size = compat_kargs.stack_size; + kargs.tls = compat_kargs.tls; +return _do_fork (flags, &kargs); }#endif /* CONFIG_COMPAT */ #endif /* CONFIG_CLONE4 */
Signature · 1 line
2.1.4
PATCH v2 5/7 clone4: Add a CLONE_AUTOREAP flag to automatically reap the child process
In reply to Josh TriplettIf a process launches a child process with the notification signal set to SIGCHLD (e.g. with fork()), and then the parent process either ignores SIGCHLD or sets a handler with SA_NOCLDWAIT, the child process will get automatically reaped without waiting for the parent to wait on it.
However, there's currently no way to get the same autoreaping behavior if the signal is not set to SIGCHLD, including in particular if the signal is set to 0 to disable notification. Furthermore, the code launching the child process may not own process-wide signal handling for the parent process.
Add a CLONE_AUTOREAP flag to request this behavior unconditionally, regardless of the notification signal or the state of the parent process's signal handling when the process exits.
This is particularly useful for libraries that want to launch unattended child processes without interfering with the calling process's signal handling or wait loop.
--- include/linux/sched.h | 2 ++ include/uapi/linux/sched.h | 7 ++++++- kernel/fork.c | 2 ++ kernel/signal.c | 2 ++ 4 files changed, 12 insertions(+), 1 deletion(-)
include/linux/sched.h +2 -0
@@ -1372,6 +1372,8 @@ struct task_struct { unsigned memcg_kmem_skip_account:1;#endif + unsigned autoreap:1; /* Do not become a zombie on exit */ +unsigned long atomic_flags;/* Flags needing atomic access. */ struct restart_block restart_block;
include/uapi/linux/sched.h +6 -1
@@ -37,13 +37,18 @@#define CLONE_DETACHED 0x00400000#define CLONE_STOPPED 0x02000000 +/* +* Flags that only work with clone4. +*/ +#define CLONE_AUTOREAP 0x00001000/* Automatically reap the process */ +#ifdef __KERNEL__/* * Valid flags for clone and for clone4. Kept in this file next to the flag * list above, but not exposed to userspace. */ #define CLONE_VALID_FLAGS (0xffffffffULL & ~(CLONE_PID | CLONE_DETACHED | CLONE_STOPPED)) -#define CLONE4_VALID_FLAGS CLONE_VALID_FLAGS +#define CLONE4_VALID_FLAGS (CLONE_VALID_FLAGS | CLONE_AUTOREAP)#endif /* __KERNEL__ */ /*
kernel/fork.c +2 -0
@@ -1461,6 +1461,8 @@ static struct task_struct *copy_process(u64 clone_flags, p->tgid = p->pid; } + p->autoreap = !!(clone_flags &CLONE_AUTOREAP ); + p->nr_dirtied =0 ; p->nr_dirtied_pause =128 >> (PAGE_SHIFT -10 ); p->dirty_paused_when =0 ;
kernel/signal.c +2 -0
@@ -1702,6 +1702,8 @@ bool do_notify_parent(struct task_struct *tsk, int sig)if (psig->action[SIGCHLD -1 ].sa.sa_handler ==SIG_IGN ) sig =0 ; } +if (!tsk->ptrace && tsk->autoreap) + autoreap = true;if (valid_signal (sig) && sig)__group_send_sig_info (sig, &info, tsk->parent);__wake_up_parent (tsk, tsk->parent);
Signature · 1 line
2.1.4
Re: [PATCH v2 5/7] clone4: Add a CLONE_AUTOREAP flag to automatically reap the child process
Quoted 2 lines · Josh Triplett
Add a
CLONE_AUTOREAPflag to request this behavior unconditionally,
Yes, CLONE_AUTOREAP is much better. And I agree (mostly) with that we should rely on do_notify_parent().
Howver the patch still doesn't look right. First of all, ->autoreap should be per-process, not per-thread. And there are ptrace/mt issues, it seems. Just for example, we should avoid EXIT_TRACE if autoreap in wait_task_zombie() even if we are going to re-notify parent.
Yes... and other problems with ptrace. So let me nack this patch for the moment ;) But let me repeat that personally I agree with this change "in general".
EXCEPT: do we really want SIGCHLD from the exiting child? I think we do not. I won't really argue though, but this should be discussed and documented. IIUC, with your patch it is still sent.
Josh, please give me some time to think and re-check, I'll write another email next week. I am not sure this is really needed, but it seems to me that we need the preparation patch to make this change clear/simple.
Oleg.
Re: [PATCH v2 5/7] clone4: Add a CLONE_AUTOREAP flag to automatically reap the child process
Quoted 8 lines · Oleg Nesterov
Quoted 1 line · Josh Triplett
Add a
CLONE_AUTOREAPflag to request this behavior unconditionally,Yes,
CLONE_AUTOREAPis much better. And I agree (mostly) with that we should rely ondo_notify_parent().Howver the patch still doesn't look right. First of all, ->autoreap should be per-process, not per-thread.
Ah, you're thinking of the case where the parent process launches a child with CLONE_AUTOREAP, that child process launches siblings with CLONE_THREAD and without CLONE_AUTOREAP, and one of those siblings is the last to exit? That seems easy enough to handle: instead of setting ->autoreap unconditionally in copy_process, I can set it only in the non-CLONE_THREAD case, and otherwise let it inherit. Then every task in the group will have the same value for autoreap.
(As an aside, what *is* the use case for CLONE_PARENT without CLONE_THREAD?)
Quoted 3 lines
And there are ptrace/mt issues,
it seems. Just for example, we should avoidEXIT_TRACEif autoreap inwait_task_zombie()even if we are going to re-notify parent.
I don't see how EXIT_TRACE can happen in wait_task_zombie if autoreap is set. wait_task_zombie does a cmpxchg with exit_state and doesn't proceed unless exit_state was EXIT_ZOMBIE, and I don't see how we can ever reach the EXIT_ZOMBIE state if autoreap.
Quoted 3 lines
EXCEPT: do we really want SIGCHLD from the exiting child? I think we do not. I won't really argue though, but this should be discussed and documented. IIUC, with your patch it is still sent.
I think we do, yes. The caller of clone can already specify what signal they want, including no signal at all. If they specify a signal (SIGCHLD or otherwise) along with CLONE_AUTOREAP, we can send that signal. I don't think that causes any particular problem.
That's the same semantic you'd get if you have a SIGCHLD handler withSA_NOCLDWAIT: you'd still get the signal, even though you don't need to (and can't) wait on the child process.
Quoted 3 lines
Josh, please give me some time to think and re-check, I'll write another email next week. I am not sure this is really needed, but it seems to me that we need the preparation patch to make this change clear/simple.
I'd appreciate any feedback you can offer on this series, including any potential subtle interactions with ptrace.
- Josh Triplett
Re: [PATCH v2 5/7] clone4: Add a CLONE_AUTOREAP flag to automatically reap the child process
Quoted 5 lines · Josh Triplett
Quoted 3 lines · Oleg Nesterov
Quoted 1 line · Josh Triplett
Add a
CLONE_AUTOREAPflag to request this behavior unconditionally,
Quoted 8 lines
Quoted 5 lines
Yes,
CLONE_AUTOREAPis much better. And I agree (mostly) with that we should rely ondo_notify_parent().Howver the patch still doesn't look right. First of all, ->autoreap should be per-process, not per-thread.
Ah, you're thinking of the case where the parent process launches a
...
Not really, although we probably need more sanity checks.
It should be per-process simply because this "autoreap" affects the whole process. And the sub-threads are already "autoreap". And these 2 autoreap's semantics differ, we should not confuse them.
Quoted 2 lines
(As an aside, what *is* the use case for
CLONE_PARENTwithoutCLONE_THREAD?)
To me CLONE_PARENT is another historical mistake and the source of misc problems ;)
Quoted 8 lines
Quoted 3 lines
And there are ptrace/mt issues,
it seems. Just for example, we should avoidEXIT_TRACEif autoreap inwait_task_zombie()even if we are going to re-notify parent.I don't see how
EXIT_TRACEcan happen inwait_task_zombieif autoreap is set.wait_task_zombiedoes a cmpxchg withexit_stateand doesn't proceed unlessexit_statewasEXIT_ZOMBIE, and I don't see how we can ever reach theEXIT_ZOMBIEstate if autoreap.
Because you again forgot about ptrace ;)
Josh. Let me try to summarise this later when I have time. Again, I am not sure, perhaps this is even simpler than I currently think. And let me apologize in advance, most probably I will be busy tomorrow.
Quoted 8 lines
Quoted 3 lines
EXCEPT: do we really want SIGCHLD from the exiting child? I think we do not. I won't really argue though, but this should be discussed and documented. IIUC, with your patch it is still sent.
I think we do, yes. The caller of clone can already specify what signal they want, including no signal at all. If they specify a signal (SIGCHLD or otherwise) along with
CLONE_AUTOREAP, we can send that signal.
OK. Agreed.
Oleg.
Re: [PATCH v2 5/7] clone4: Add a CLONE_AUTOREAP flag to automatically reap the child process
Quoted 11 lines · Oleg Nesterov
Quoted 10 lines · Josh Triplett
Quoted 8 lines · Oleg Nesterov
Quoted 1 line · Josh Triplett
Add a
CLONE_AUTOREAPflag to request this behavior unconditionally,Yes,
CLONE_AUTOREAPis much better. And I agree (mostly) with that we should rely ondo_notify_parent().Howver the patch still doesn't look right. First of all, ->autoreap should be per-process, not per-thread.
Quoted 8 lines
Quoted 2 lines
Ah, you're thinking of the case where the parent process launches a
...Not really, although we probably need more sanity checks.
It should be per-process simply because this "autoreap" affects the whole process. And the sub-threads are already "autoreap". And these 2 autoreap's semantics differ, we should not confuse them.
Will the approach I suggested, of having clones with CLONE_THREAD inherit the autoreap value rather than setting it from CLONE_AUTOREAP, implement the semantics you're looking for?
Also, are you suggesting that CLONE_AUTOREAP with CLONE_THREAD should produce -EINVAL, or just that it should be ignored?
Quoted 5 lines
Quoted 2 lines
(As an aside, what *is* the use case for
CLONE_PARENTwithoutCLONE_THREAD?)To me
CLONE_PARENTis another historical mistake and the source of misc problems ;)
I kinda figured. :)
Quoted 9 lines
Quoted 8 lines
Quoted 3 lines
And there are ptrace/mt issues,
it seems. Just for example, we should avoidEXIT_TRACEif autoreap inwait_task_zombie()even if we are going to re-notify parent.I don't see how
EXIT_TRACEcan happen inwait_task_zombieif autoreap is set.wait_task_zombiedoes a cmpxchg withexit_stateand doesn't proceed unlessexit_statewasEXIT_ZOMBIE, and I don't see how we can ever reach theEXIT_ZOMBIEstate if autoreap.
Quoted 5 lines
Because you again forgot about ptrace ;)
Josh. Let me try to summarise this later when I have time. Again, I am not sure, perhaps this is even simpler than I currently think. And let me apologize in advance, most probably I will be busy tomorrow.
I look forward to your later review and feedback.
- Josh Triplett
Re: [PATCH v2 5/7] clone4: Add a CLONE_AUTOREAP flag to automatically reap the child process
Josh,
I am really sorry for delay.
Quoted 10 lines · Josh Triplett
Quoted 3 lines · Oleg Nesterov
It should be per-process simply because this "autoreap" affects the whole process. And the sub-threads are already "autoreap". And these 2 autoreap's semantics differ, we should not confuse them.
Will the approach I suggested, of having clones with
CLONE_THREADinherit the autoreap value rather than setting it fromCLONE_AUTOREAP, implement the semantics you're looking for?
Not sure I understand... CLONE_THREAD should not inherit the autoreap. A sub-thread is always autoreapable.
Quoted 2 lines
Also, are you suggesting that
CLONE_AUTOREAPwithCLONE_THREADshould produce-EINVAL, or just that it should be ignored?
Yes, I think CLONE_AUTOREAP | CLONE_THREAD should return -EINVAL. But this all is minor...
The main problem is how/when we should check this "autoreap" without making this code even more ugly.
I still think we need a preparation patch. I tried to make it today but failed. Will try again on weekend...
Note that we can't solely rely on do_notify_parent() which (with your patch) correctly checks !ptrace && autoreap.
Just for example. Please look at __ptrace_detach(). Note that if we add CLONE_AUTOREAP this needs a fix in any case. The tracee can be "autoreap" but zombie, because "autoreap" should be ignored until the tracer detaches. But the "same_thread_group" should not call do_notify_parent() again. So this needs another check.
And let me quote our discussion from the previous email:
> > EXCEPT: do we really want SIGCHLD from the exiting child? I think we > > do not. I won't really argue though, but this should be discussed and > > documented. IIUC, with your patch it is still sent.
>
> I think we do, yes. The caller of clone can already specify what signal > they want, including no signal at all. If they specify a signal > (SIGCHLD or otherwise) along with CLONE_AUTOREAP, we can send that > signal.
OK. Agreed.
Yes, I agree...
But the changes in __ptrace_detach() depend on whether we need to send a signal or not. Either way the changle is simple, but looks ugly. It would be nice to cleanup this somehow.
Also. I forgot that the kernel always resets ->exit_signal to SIGCHLD on exec or reparenting. Reparenting is probably fine. But what about exec? Should it keep ->exit_signal == 0 if "autoreap" ? I think it should not, to avoid the strange special case.
Quoted 10 lines
Quoted 10 lines
Quoted 8 lines
Quoted 3 lines
And there are ptrace/mt issues,
it seems. Just for example, we should avoidEXIT_TRACEif autoreap inwait_task_zombie()even if we are going to re-notify parent.I don't see how
EXIT_TRACEcan happen inwait_task_zombieif autoreap is set.wait_task_zombiedoes a cmpxchg withexit_stateand doesn't proceed unlessexit_statewasEXIT_ZOMBIE, and I don't see how we can ever reach theEXIT_ZOMBIEstate if autoreap.Because you again forgot about ptrace ;)
And this too asks for preparation before CLONE_AUTOREAP...
So I'll try to think about this all again on weekend. I'll try very much to not disappear again ;)
Oleg.
Re: [PATCH v2 5/7] clone4: Add a CLONE_AUTOREAP flag to automatically reap the child process
Quoted 4 lines · On Friday 20 March 2015 19:14:04 Oleg Nesterov wrote:
Also. I forgot that the kernel always resets ->exit_signal to SIGCHLD on exec or reparenting. Reparenting is probably fine. But what about exec? Should it keep ->exit_signal == 0 if "autoreap" ? I think it should not, to avoid the strange special case.
Not delivering any signal was the objective of this patch series, so yes exit_signal == 0 should survive an exec and even re-exec.
Signature · 2 lines
Thiago Macieira - thiago.macieira (AT) intel.com Software Architect - Intel Open Source Technology Center
Re: [PATCH v2 5/7] clone4: Add a CLONE_AUTOREAP flag to automatically reap the child process
Quoted 9 lines · Thiago Macieira
Quoted 4 lines · On Friday 20 March 2015 19:14:04 Oleg Nesterov wrote:
Also. I forgot that the kernel always resets ->exit_signal to SIGCHLD on exec or reparenting. Reparenting is probably fine. But what about exec? Should it keep ->exit_signal == 0 if "autoreap" ? I think it should not, to avoid the strange special case.
Not delivering any signal was the objective of this patch series, so yes
exit_signal== 0 should survive an exec and even re-exec.
OK, but then perhaps we should never send SIGCHLD (on exit) if "autoreap", to make the logic simple.
And copy_process() should probably do
if ((clone_flags & CSIGNAL) && (clone_flags && CLONE_AUTOREAP)) return -EINVAL;
so that we still can change this behaviour later.
Oleg.
Re: [PATCH v2 5/7] clone4: Add a CLONE_AUTOREAP flag to automatically reap the child process
Quoted 14 lines · Oleg Nesterov
Quoted 9 lines · Thiago Macieira
Quoted 4 lines · On Friday 20 March 2015 19:14:04 Oleg Nesterov wrote:
Also. I forgot that the kernel always resets ->exit_signal to SIGCHLD on exec or reparenting. Reparenting is probably fine. But what about exec? Should it keep ->exit_signal == 0 if "autoreap" ? I think it should not, to avoid the strange special case.
Not delivering any signal was the objective of this patch series, so yes
exit_signal== 0 should survive an exec and even re-exec.OK, but then perhaps we should never send SIGCHLD (on exit) if "autoreap", to make the logic simple.
Quoted 6 lines
And
copy_process()should probably doif ((clone_flags & CSIGNAL) && (clone_flags && CLONE_AUTOREAP)) return -EINVAL;so that we still can change this behaviour later.
I'm fine with that, as it would handle the particular use case we care about.
However, the reset-signal-on-reparent thing might still make sense, particularly for the ptrace-reparent case (less so for the reparent-to-child-reaper case).
- Josh Triplett
PATCH v2 6/7 signal: Factor out a helper function to process task_struct exit_code
In reply to Josh Triplettdo_notify_parent includes the code to convert the exit_code field of struct task_struct to the code and status fields that accompany SIGCHLD. Factor that out into a new helper function task_exit_code_status, to allow other methods of task exit notification to share that code.
--- include/linux/sched.h | 1 + kernel/signal.c | 24 +++++++++++++++--------- 2 files changed, 16 insertions(+), 9 deletions(-)
include/linux/sched.h +1 -0
@@ -2388,6 +2388,7 @@ extern int kill_pid_info_as_cred(int, struct siginfo *, struct pid *,extern int kill_pgrp (struct pid *pid,int sig,int priv);extern int kill_pid (struct pid *pid,int sig,int priv);extern int kill_proc_info (int ,struct siginfo *,pid_t ); +extern void task_exit_code_status (int exit_code,s32 *code,s32 *status);extern __must_check bool do_notify_parent (struct task_struct *,int );extern void __wake_up_parent (struct task_struct *p,struct task_struct *parent);extern void force_sig (int ,struct task_struct *);
kernel/signal.c +15 -9
@@ -1613,6 +1613,20 @@ ret:return ret; } +/* Translate exit_code to code and status. */ +void task_exit_code_status (int exit_code,s32 *code,s32 *status) +{ + *status = exit_code &0x7f ; +if (exit_code &0x80 ) + *code =CLD_DUMPED ; +else if (exit_code &0x7f ) + *code =CLD_KILLED ; +else { + *code =CLD_EXITED ; + *status = exit_code >>8 ; + } +} +/* * Let a parent know about the death of a child. * For a stopped/continued status change, use do_notify_parent_cldstop instead.
@@ -1668,15 +1682,7 @@ bool do_notify_parent(struct task_struct *tsk, int sig) info.si_utime =cputime_to_clock_t (utime + tsk->signal->utime); info.si_stime =cputime_to_clock_t (stime + tsk->signal->stime); - info.si_status = tsk->exit_code &0x7f ; -if (tsk->exit_code &0x80 ) - info.si_code =CLD_DUMPED ; -else if (tsk->exit_code &0x7f ) - info.si_code =CLD_KILLED ; -else { - info.si_code =CLD_EXITED ; - info.si_status = tsk->exit_code >>8 ; - } +task_exit_code_status (tsk->exit_code, &info.si_code, &info.si_status); psig = tsk->parent->sighand;spin_lock_irqsave (&psig->siglock, flags);
Signature · 1 line
2.1.4
PATCH v2 7/7 clone4: Add a CLONE_FD flag to get task exit notification via fd
In reply to Josh TriplettWhen passed CLONE_FD, clone4 hands the caller a file descriptor referring to the new process. When the new process exits, the file descriptor becomes readable, producing a structure containing the exit status, exit code, and user/system times. The file descriptor also works in epoll, poll, and select.
This allows libraries to safely launch and manage child processes on behalf of a caller, without taking over or interfering with process-wide signal handling. Without this, such a library would need to take over or cooperate with the entire process's SIGCHLD handling, either via a signal handler or a signalfd.
--- include/linux/compat.h | 2 + include/linux/sched.h | 5 ++ include/uapi/linux/sched.h | 16 +++++- init/Kconfig | 11 +++++ kernel/Makefile | 1 + kernel/clonefd.c | 121 +++++++++++++++++++++++++++++++++++++++++++++ kernel/clonefd.h | 32 ++++++++++++ kernel/exit.c | 4 ++ kernel/fork.c | 22 +++++++-- 9 files changed, 209 insertions(+), 5 deletions(-) create mode 100644 kernel/clonefd.c create mode 100644 kernel/clonefd.h
include/linux/compat.h +2 -0
@@ -299,6 +299,8 @@ struct compat_clone4_args {compat_ulong_t stack_start;compat_ulong_t stack_size;compat_ulong_t tls; +compat_uptr_t clonefd; +u32 clonefd_flags; };struct compat_statfs ;
include/linux/sched.h +5 -0
@@ -1374,6 +1374,11 @@ struct task_struct { unsigned autoreap:1; /* Do not become a zombie on exit */ +#ifdef CONFIG_CLONEFD + unsigned clonefd:1; /* Notify clonefd_wqh on exit */ +wait_queue_head_t clonefd_wqh; +#endif +unsigned long atomic_flags;/* Flags needing atomic access. */ struct restart_block restart_block;
include/uapi/linux/sched.h +15 -1
@@ -41,6 +41,7 @@* Flags that only work with clone4. */ #define CLONE_AUTOREAP 0x00001000/* Automatically reap the process */ +#define CLONE_FD 0x00400000/* Signal exit via file descriptor */ #ifdef __KERNEL__/*
@@ -48,10 +49,21 @@* list above, but not exposed to userspace. */ #define CLONE_VALID_FLAGS (0xffffffffULL & ~(CLONE_PID | CLONE_DETACHED | CLONE_STOPPED)) -#define CLONE4_VALID_FLAGS (CLONE_VALID_FLAGS | CLONE_AUTOREAP) +#define CLONE4_VALID_FLAGS (CLONE_VALID_FLAGS | CLONE_AUTOREAP | \ + (IS_ENABLED(CONFIG_CLONEFD) ? CLONE_FD : 0))#endif /* __KERNEL__ */ /* +* Structure read from CLONE_FD file descriptor after process exits +*/ +struct clonefd_info { +__s32 code; +__s32 status; +__u64 utime; +__u64 stime; +}; + +/* * Structure passed to clone4 for additional arguments. Initialized to 0, * then overwritten with arguments from userspace, so arguments not supplied by * userspace will remain 0. New versions of the kernel may safely append new
@@ -63,6 +75,8 @@ struct clone4_args {__kernel_ulong_t stack_start;__kernel_ulong_t stack_size;__kernel_ulong_t tls; +int __user *clonefd; +__u32 clonefd_flags; };/*
init/Kconfig +11 -0
@@ -1521,6 +1521,17 @@ config CLONE4 If unsure, say Y. +config CLONEFD + bool "Enable CLONE_FD flag for clone4()" if EXPERT + depends on CLONE4 + select ANON_INODES + default y + help + Enable the CLONE_FD flag for clone4(), which creates a file descriptor + to receive child exit events rather than receiving a signal. + + If unsure, say Y. + # syscall, maps, verifier config BPF_SYSCALL bool "Enable bpf() system call" if EXPERT
kernel/Makefile +1 -0
@@ -29,6 +29,7 @@ obj-y += rcu/ obj-y += livepatch/ obj-$(CONFIG_CHECKPOINT_RESTORE) += kcmp.o +obj-$(CONFIG_CLONEFD) += clonefd.o obj-$(CONFIG_FREEZER) += freezer.o obj-$(CONFIG_PROFILING) += profile.o obj-$(CONFIG_STACKTRACE) += stacktrace.o
kernel/clonefd.c new file +121 -0
121 lines not on this page · raw message
kernel/clonefd.h new file +32 -0
@@ -0,0 +1,32 @@ +/* +* Support functions for CLONE_FD +* +* Copyright (c) 2015 Intel Corporation +* Original authors: Josh Triplett <[email protected]> +* Thiago Macieira <[email protected]> +*/ +#pragma once + +#include <linux/sched.h> + +#ifdef CONFIG_CLONEFD +struct clonefd_setup { +int fd; +struct file *file; +}; +int clonefd_do_clone (u64 clone_flags,struct task_struct *p, +struct clone4_args *args,struct clonefd_setup *setup); +void clonefd_cleanup_failed_clone (struct clonefd_setup *setup); +void clonefd_install_fd (struct clone4_args *args,struct clonefd_setup *setup); +void clonefd_do_notify (struct task_struct *p); +#else /* CONFIG_CLONEFD */ +struct clonefd_setup {}; +static inline int clonefd_do_clone (u64 clone_flags,struct task_struct *p, +struct clone4_args *args,struct clonefd_setup *setup) +{ +return 0 ; +} +static inline void clonefd_cleanup_failed_clone (struct clonefd_setup *setup) {} +static inline void clonefd_install_fd (struct clone4_args *args,struct clonefd_setup *setup) {} +static inline void clonefd_do_notify (struct task_struct *p) {} +#endif /* CONFIG_CLONEFD */
kernel/exit.c +4 -0
@@ -59,6 +59,8 @@#include <asm/pgtable.h> #include <asm/mmu_context.h> +#include "clonefd.h" +static void exit_mm (struct task_struct *tsk);static void __unhash_process (struct task_struct *p,bool group_dead)
@@ -615,6 +617,8 @@ static void exit_notify(struct task_struct *tsk, int group_dead)if (tsk->exit_state ==EXIT_DEAD )list_add (&tsk->ptrace_entry, &dead); +clonefd_do_notify (tsk); +/* mt-exec, de_thread() is waiting for group leader */ if (unlikely (tsk->signal->notify_count <0 ))wake_up_process (tsk->signal->group_exit_task);
kernel/fork.c +18 -4
@@ -87,6 +87,8 @@#define CREATE_TRACE_POINTS #include <trace/events/task.h> +#include "clonefd.h" +/* * Protected counters by write_lock_irq(&tasklist_lock) */
@@ -1190,7 +1192,8 @@ init_task_pid(struct task_struct *task, enum pid_type type, struct pid *pid)static struct task_struct *copy_process (u64 clone_flags,struct clone4_args *args,struct pid *pid, -int trace) +int trace, +struct clonefd_setup *clonefd_setup) {int retval;struct task_struct *p;
@@ -1413,6 +1416,10 @@ static struct task_struct *copy_process(u64 clone_flags, goto bad_fork_cleanup_io; } + retval =clonefd_do_clone (clone_flags, p, args, clonefd_setup); +if (retval) + goto bad_fork_free_pid; + p->set_child_tid = (clone_flags &CLONE_CHILD_SETTID ) ? args->ctid :NULL ;/* * Clear TID on mm_release()?
@@ -1507,7 +1514,7 @@ static struct task_struct *copy_process(u64 clone_flags,spin_unlock (¤t->sighand->siglock);write_unlock_irq (&tasklist_lock); retval = -ERESTARTNOINTR ; - goto bad_fork_free_pid; + goto bad_fork_cleanup_clonefd; }if (likely (p->pid)) {
@@ -1559,6 +1566,8 @@ static struct task_struct *copy_process(u64 clone_flags,return p; +bad_fork_cleanup_clonefd: +clonefd_cleanup_failed_clone (clonefd_setup); bad_fork_free_pid:if (pid != &init_struct_pid)free_pid (pid);
@@ -1617,7 +1626,7 @@ struct task_struct *fork_idle(int cpu) {struct task_struct *task;struct clone4_args args = {}; - task =copy_process (CLONE_VM , &args, &init_struct_pid,0 ); + task =copy_process (CLONE_VM , &args, &init_struct_pid,0 ,NULL );if (!IS_ERR (task)) {init_idle_pids (task->pids);init_idle (task, cpu);
@@ -1637,6 +1646,7 @@ static long _do_fork(u64 clone_flags, struct clone4_args *args)struct task_struct *p;int trace =0 ;long nr; +struct clonefd_setup clonefd_setup = {};/* * Determine whether and which event to report to ptracer. When
@@ -1656,7 +1666,7 @@ static long _do_fork(u64 clone_flags, struct clone4_args *args) trace =0 ; } - p =copy_process (clone_flags, args,NULL , trace); + p =copy_process (clone_flags, args,NULL , trace, &clonefd_setup);/* * Do this prior waking up the new thread - the thread pointer * might get invalid after that point, if the thread exits quickly.
@@ -1679,6 +1689,8 @@ static long _do_fork(u64 clone_flags, struct clone4_args *args)get_task_struct (p); } +clonefd_install_fd (args, &clonefd_setup); +wake_up_new_task (p);/* forking complete and child started to run, tell ptracer */
@@ -1822,6 +1834,8 @@ COMPAT_SYSCALL_DEFINE4(clone4, unsigned, flags_high, unsigned, flags_low, kargs.stack_start = compat_kargs.stack_start; kargs.stack_size = compat_kargs.stack_size; kargs.tls = compat_kargs.tls; + kargs.clonefd =compat_ptr (compat_kargs.clonefd); + kargs.clonefd_flags = compat_kargs.clonefd_flags; return _do_fork(flags, &kargs); }#endif /* CONFIG_COMPAT */
Signature · 1 line
2.1.4
Re: [PATCH v2 7/7] clone4: Add a CLONE_FD flag to get task exit notification via fd
Quoted 19 lines · Josh Triplett
include/linux/compat.h +2
-0@@ -299,6 +299,8 @@ struct compat_clone4_args {compat_ulong_t stack_start;compat_ulong_t stack_size;compat_ulong_t tls; +compat_uptr_t clonefd; +u32 clonefd_flags; };struct compat_statfs ;include/linux/sched.h +0
-0@@ -1374,6 +1374,11 @@ struct task_struct {
include/linux/sched.h struct task_struct { · line 1384
Quoted 10 lines
include/linux/sched.h +5
-0unsigned autoreap:1; /* Do not become a zombie on exit */ +#ifdef CONFIG_CLONEFD + unsigned clonefd:1; /* Notify clonefd_wqh on exit */ +wait_queue_head_t clonefd_wqh; +#endif +unsigned long atomic_flags;/* Flags needing atomic access. */ struct restart_block restart_block;
Idle thought: are there any concerns about the occupancy impact of adding a wait_queue_head to every task_struct, whether it has a clonefd or not?
I guess we could reduce the size somewhat by just storing a struct file *clonefd_file in the task, and then have a separate structure (with the wqh and a task_struct*) referenced by file->private_data. Not sure whether the added complication would be worthwhile, though.
Quoted 56 lines
kernel/clonefd.c new file +50
-0@@ -0,0 +1,121 @@ +/* +* Support functions for CLONE_FD +* +* Copyright (c) 2015 Intel Corporation +* Original authors: Josh Triplett <[email protected]> +* Thiago Macieira <[email protected]> +*/ +#include <linux/anon_inodes.h> +#include <linux/file.h> +#include <linux/fs.h> +#include <linux/poll.h> +#include <linux/slab.h> +#include "clonefd.h" + +static int clonefd_release (struct inode *inode,struct file *file) +{ +put_task_struct (file->private_data); +return 0 ; +} + +static unsigned int clonefd_poll (struct file *file,poll_table *wait) +{ +struct task_struct *p = file->private_data; +poll_wait (file, &p->clonefd_wqh, wait); +return p->exit_state ? (POLLIN |POLLRDNORM |POLLHUP ) :0 ; +} + +static ssize_t clonefd_read (struct file *file,char __user *buf,size_t count,loff_t *ppos) +{ +struct task_struct *p = file->private_data; +int ret =0 ; + +/* EOF after first read */ +if (*ppos) +return 0 ; + +if (file->f_flags &O_NONBLOCK ) + ret = -EAGAIN ; +else + ret =wait_event_interruptible (p->clonefd_wqh, p->exit_state); + +if (p->exit_state) { +struct clonefd_info info = {}; +cputime_t utime, stime; +task_exit_code_status (p->exit_code, &info.code, &info.status); + info.code &= ~__SI_MASK; +task_cputime (p, &utime, &stime); + info.utime =cputime_to_clock_t (utime + p->signal->utime); + info.stime =cputime_to_clock_t (stime + p->signal->stime); + ret =simple_read_from_buffer (buf, count, ppos, &info,sizeof (info));
kernel/clonefd.c line 60
Quoted 10 lines
kernel/clonefd.c new file +10
-0+ } +return ret; +} + +static struct file_operations clonefd_fops = { + .release = clonefd_release, + .poll = clonefd_poll, + .read = clonefd_read, + .llseek = no_llseek, +};
It might be nice to include a show_fdinfo() implementation that shows (say) the pid that the clonefd refers to. E.g. something like:
static void clonefd_show_fdinfo(struct seq_file *m, struct file *file)
{
struct task_struct *p = file->private_data;
seq_printf(m, "tid:\t%d\n", task_tgid_vnr(p));
}Quoted 13 lines
kernel/clonefd.c new file +13
-0+ +/* Do process exit notification for clonefd. */ +void clonefd_do_notify (struct task_struct *p) +{ +if (p->clonefd) +wake_up_all (&p->clonefd_wqh); +} + +/* Handle the CLONE_FD case for copy_process. */ +int clonefd_do_clone (u64 clone_flags,struct task_struct *p, +struct clone4_args *args,struct clonefd_setup *setup) +{ +int flags;
kernel/clonefd.c line 83
Quoted 10 lines
kernel/clonefd.c new file +10
-0+struct file *file; +int fd; + + p->clonefd = !!(clone_flags &CLONE_FD ); +if (!p->clonefd) +return 0 ; + +if (args->clonefd_flags & ~(O_CLOEXEC |O_NONBLOCK )) +return -EINVAL ; +
Maybe also check for (args->clonefd == NULL) in advance, and return -EINVAL or -EFAULT?
Quoted 38 lines
kernel/clonefd.c new file +38
-0+init_waitqueue_head (&p->clonefd_wqh); + +get_task_struct (p); + flags =O_RDONLY |FMODE_ATOMIC_POS | args->clonefd_flags; + file =anon_inode_getfile ("[process]" , &clonefd_fops, p, flags); +if (IS_ERR (file)) { +put_task_struct (p); +return PTR_ERR (file); + } + + fd =get_unused_fd_flags (flags); +if (fd <0 ) { +fput (file); +return fd; + } + + setup->fd = fd; + setup->file = file; +return 0 ; +} + +/* Clean up clonefd information after a partially complete clone */ +void clonefd_cleanup_failed_clone (struct clonefd_setup *setup) +{ +if (setup->file) { +put_unused_fd (setup->fd); +fput (setup->file); + } +} + +/* Finish setting up the clonefd */ +void clonefd_install_fd (struct clone4_args *args,struct clonefd_setup *setup) +{ +if (setup->file) { +fd_install (setup->fd, setup->file); +put_user (setup->fd, args->clonefd); + } +}
Re: [PATCH v2 7/7] clone4: Add a CLONE_FD flag to get task exit notification via fd
Quoted 22 lines · David Drysdale
Quoted 6 lines · Josh Triplett
include/linux/sched.h +0
-0@@ -1374,6 +1374,11 @@ struct task_struct {include/linux/sched.h struct task_struct { · line 1384
Quoted 10 lines
include/linux/sched.h +5
-0unsigned autoreap:1; /* Do not become a zombie on exit */ +#ifdef CONFIG_CLONEFD + unsigned clonefd:1; /* Notify clonefd_wqh on exit */ +wait_queue_head_t clonefd_wqh; +#endif +unsigned long atomic_flags;/* Flags needing atomic access. */ struct restart_block restart_block;Idle thought: are there any concerns about the occupancy impact of adding a
wait_queue_headto everytask_struct, whether it has a clonefd or not?
Quoted 5 lines
I guess we could reduce the size somewhat by just storing a struct file
*clonefd_filein the task, and then have a separate structure (with the wqh and a task_struct*) referenced byfile->private_data. Not sure whether the added complication would be worthwhile, though.
My original patches did exactly that (minus the reference back to the task_struct). However, there are a couple of problems with that approach. First, it assumes that a task_struct has only a single file referencing it, but in the future I'd like to support obtaining a clonefd for an existing task. Second, the task_struct really shouldn't have a reference to the actual struct file, when it only needs the wait_queue_head_t.
Also, AFAICT a wait_queue_head_t is normally (in the absence of kernel lock debugging options) the size of two pointers. Adding an indirection and an extra allocation to change that to the size of one pointer seems iffy, especially when looking at the rest of what's directly in task_struct that's far larger.
Quoted 67 lines
Quoted 53 lines
kernel/clonefd.c new file +50
-0@@ -0,0 +1,121 @@ +/* +* Support functions for CLONE_FD +* +* Copyright (c) 2015 Intel Corporation +* Original authors: Josh Triplett <[email protected]> +* Thiago Macieira <[email protected]> +*/ +#include <linux/anon_inodes.h> +#include <linux/file.h> +#include <linux/fs.h> +#include <linux/poll.h> +#include <linux/slab.h> +#include "clonefd.h" + +static int clonefd_release (struct inode *inode,struct file *file) +{ +put_task_struct (file->private_data); +return 0 ; +} + +static unsigned int clonefd_poll (struct file *file,poll_table *wait) +{ +struct task_struct *p = file->private_data; +poll_wait (file, &p->clonefd_wqh, wait); +return p->exit_state ? (POLLIN |POLLRDNORM |POLLHUP ) :0 ; +} + +static ssize_t clonefd_read (struct file *file,char __user *buf,size_t count,loff_t *ppos) +{ +struct task_struct *p = file->private_data; +int ret =0 ; + +/* EOF after first read */ +if (*ppos) +return 0 ; + +if (file->f_flags &O_NONBLOCK ) + ret = -EAGAIN ; +else + ret =wait_event_interruptible (p->clonefd_wqh, p->exit_state); + +if (p->exit_state) { +struct clonefd_info info = {}; +cputime_t utime, stime; +task_exit_code_status (p->exit_code, &info.code, &info.status); + info.code &= ~__SI_MASK; +task_cputime (p, &utime, &stime); + info.utime =cputime_to_clock_t (utime + p->signal->utime); + info.stime =cputime_to_clock_t (stime + p->signal->stime); + ret =simple_read_from_buffer (buf, count, ppos, &info,sizeof (info));kernel/clonefd.c line 60
Quoted 10 lines
kernel/clonefd.c new file +10
-0+ } +return ret; +} + +static struct file_operations clonefd_fops = { + .release = clonefd_release, + .poll = clonefd_poll, + .read = clonefd_read, + .llseek = no_llseek, +};It might be nice to include a
show_fdinfo()implementation that shows (say) the pid that the clonefd refers to. E.g. something like:
Quoted 6 lines
static void
clonefd_show_fdinfo(struct seq_file *m, struct file *file){ struct task_struct *p = file->private_data; seq_printf(m, "tid:\t%d\n", task_tgid_vnr(p)); }
I thought about that, but that would add a couple of additional ifdefs (CONFIG_PROC_FS), for an informational file of minimal value. More importantly, I don't want to add that until after adding an ioctl or similar to programmatically obtain the pid from a clonefd; otherwise, someone might try to use fdinfo as the "API" to do so, which would be all kinds of awful.
So I'd prefer to add fdinfo in a future extension of clonefd, rather than in the initial patch series.
Quoted 18 lines
Quoted 18 lines
kernel/clonefd.c new file +18
-0+ +/* Do process exit notification for clonefd. */ +void clonefd_do_notify (struct task_struct *p) +{ +if (p->clonefd) +wake_up_all (&p->clonefd_wqh); +} + +/* Handle the CLONE_FD case for copy_process. */ +int clonefd_do_clone (u64 clone_flags,struct task_struct *p, +struct clone4_args *args,struct clonefd_setup *setup) +{ +int flags; +struct file *file; +int fd; + + p->clonefd = !!(clone_flags &CLONE_FD ); +if (!p->clonefd)
Quoted 8 lines
kernel/clonefd.c line 83
Quoted 5 lines
kernel/clonefd.c new file +5
-0+return 0 ; + +if (args->clonefd_flags & ~(O_CLOEXEC |O_NONBLOCK )) +return -EINVAL ; +Maybe also check for (
args->clonefd== NULL) in advance, and return-EINVALor-EFAULT?
That wouldn't be consistent with how clone treats its various other out argument pointers.
- Josh Triplett
Re: [PATCH v2 7/7] clone4: Add a CLONE_FD flag to get task exit notification via fd
In reply to Josh TriplettOn (03/15/15 01:00), Josh Triplett wrote:
[..]
Quoted 18 lines
kernel/clonefd.c new file +18
-0+ +/* Handle the CLONE_FD case for copy_process. */ +int clonefd_do_clone (u64 clone_flags,struct task_struct *p, +struct clone4_args *args,struct clonefd_setup *setup) +{ +int flags; +struct file *file; +int fd; + + p->clonefd = !!(clone_flags &CLONE_FD ); +if (!p->clonefd) +return 0 ; + +if (args->clonefd_flags & ~(O_CLOEXEC |O_NONBLOCK )) +return -EINVAL ; + +init_waitqueue_head (&p->clonefd_wqh); +
kernel/clonefd.c line 95
Quoted 10 lines
kernel/clonefd.c new file +10
-0+get_task_struct (p); + flags =O_RDONLY |FMODE_ATOMIC_POS | args->clonefd_flags; + file =anon_inode_getfile ("[process]" , &clonefd_fops, p, flags); +if (IS_ERR (file)) { +put_task_struct (p); +return PTR_ERR (file); + } + + fd =get_unused_fd_flags (flags); +if (fd <0 ) {
+ put_task_struct(p); ?
Quoted 8 lines
kernel/clonefd.c new file +8
-0+fput (file); +return fd; + } + + setup->fd = fd; + setup->file = file; +return 0 ; +}
[..]
-ss
Re: [PATCH v2 7/7] clone4: Add a CLONE_FD flag to get task exit notification via fd
Quoted 31 lines · Sergey Senozhatsky
On (03/15/15 01:00), Josh Triplett wrote:
[..]Quoted 28 lines
kernel/clonefd.c new file +28
-0+ +/* Handle the CLONE_FD case for copy_process. */ +int clonefd_do_clone (u64 clone_flags,struct task_struct *p, +struct clone4_args *args,struct clonefd_setup *setup) +{ +int flags; +struct file *file; +int fd; + + p->clonefd = !!(clone_flags &CLONE_FD ); +if (!p->clonefd) +return 0 ; + +if (args->clonefd_flags & ~(O_CLOEXEC |O_NONBLOCK )) +return -EINVAL ; + +init_waitqueue_head (&p->clonefd_wqh); + +get_task_struct (p); + flags =O_RDONLY |FMODE_ATOMIC_POS | args->clonefd_flags; + file =anon_inode_getfile ("[process]" , &clonefd_fops, p, flags); +if (IS_ERR (file)) { +put_task_struct (p); +return PTR_ERR (file); + } + + fd =get_unused_fd_flags (flags); +if (fd <0 ) {
Review
Quoted 1 line
(quoted diff) +1
-0+ put_task_struct(p); ?
No, once anon_inode_getfile has succeeded, the file owns the reference to the task_struct, so fput(file) will call the release function which calls put_task_struct. Only the failure case for anon_inode_getfile needs to call put_task_struct directly.
Quoted 3 lines
kernel/clonefd.c line 98
Quoted 3 lines
kernel/clonefd.c new file +3
-0+fput (file); +return fd; + }
- Josh Triplett
PATCH v2 1/1 clone4.2: New manpage documenting clone4(2)
In reply to Josh TriplettAlso includes new cross-reference from clone.2.
--- man2/clone.2 | 1 + man2/clone4.2 | 345 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 346 insertions(+) create mode 100644 man2/clone4.2
man2/clone.2 +1 -0
@@ -1209,6 +1209,7 @@ main(int argc, char *argv[]) } .fi .SH SEE ALSO +.BR clone4 (2), .BR fork (2), .BR futex (2), .BR getpid (2),
man2/clone4.2 new file +345 -0
345 lines not on this page · raw message
Signature · 1 line
2.1.4
Quoted 4 lines · Josh Triplett
This patch series also introduces a clone flag
CLONE_AUTOREAP, which causes the kernel to automatically reap the child process when it exits, just as it does for processes using SIGCHLD when the parent has SIGCHLD ignored or marked asSA_NOCLDSTOP.
Typo: s/SA_NOCLDSTOP/SA_NOCLDWAIT/.
- Josh Triplett
Quoted 41 lines · Josh Triplett
This patch series introduces a new clone flag,
CLONE_FD, which lets the caller receive child process exit notification via a file descriptor rather than SIGCHLD.CLONE_FDmakes it possible for libraries to safely launch and manage child processes on behalf of their caller, *without* taking over process-wide SIGCHLD handling (either via signal handler or signalfd).Note that signalfd for SIGCHLD does not suffice here, because that still receives notification for all child processes, and interferes with process-wide signal handling.
The
CLONE_FDfile descriptor uniquely identifies a process on the system in a race-free way, by holding a reference to thetask_struct. In the future, we may introduce APIs that support using process file descriptors instead of PIDs.This patch series also introduces a clone flag
CLONE_AUTOREAP, which causes the kernel to automatically reap the child process when it exits, just as it does for processes using SIGCHLD when the parent has SIGCHLD ignored or marked asSA_NOCLDSTOP.Taken together, a library can launch a process with
CLONE_FD,CLONE_AUTOREAP, and no exit signal, and completely avoid affecting either process-wide signal handling or an existing child wait loop.Introducing
CLONE_FDandCLONE_AUTOREAPrequired two additional bits of yak
shaving: Since clone has no more usable flags (with the three currently unused flags unusable because old kernels ignore them without EINVAL), also introduce a new clone4 system call with more flag bits and an extensible argument structure. And since the magic pt_regs-based syscall argument processing for clone's tls argument would otherwise prevent introducing a sane clone4 system call, fix that too.I tested the
CLONE_SETTLSchanges with a thread-local storage test program (two threads independently reading and writing a__threadvariable), on both 32-bit and 64-bit, and I observed no issues there.I tested clone4 and the new flags with several additional test programs, launching either a process or thread (in the former case using
syscall(), in the latter case by calling clone4 via assembly and returning to C), sleeping in parent and child to test the case of either exiting first, and then printing the receivedclone4_infostructure.
Quoted 15 lines
Changes in v2:
- Split out autoreaping into a separateCLONE_AUTOREAP.CLONE_FDno longer implies autoreaping and no exit signal, andCLONE_AUTOREAPdoes not affect ptracers or signal handling. Thanks to Oleg Nesterov for careful investigation and discussion on v1.
- AcceptO_CLOEXECandO_NONBLOCKvia aclonefd_flagsparameter inclone4_args. Stop overloading the low byte of the main clone flags, sinceCLONE_FDnow works with a non-zero signal.
- Return the file descriptor via an out parameter inclone4_args.
- Drop patch to exportalloc_fd;CLONE_FDnow uses the next available file descriptor, even if that's 0-2, since clone4 no longer needs to avoid ambiguity with the 0 return indicating the child process.
- Make poll on aCLONE_FDfor an exited task also return POLLHUP, for compatibility with FreeBSD's pdfork. Thanks to David Drysdale for calling attention to pdfork.
I think POLLHUP should be mentioned in the manpage (now it only mentions POLLIN).
Quoted 105 lines
- Fix typo in
squelch_clone_flags.
- Pass arguments to_do_forkandcopy_processas a structure.
- Construct the 64-bit flags in a separate variable, rather than inline in the call todo_fork.
- Fix error return forcopy_from_userfaults.
- Add the new syscall to asm-generic.
- Add ack from Andy Lutomirski to patches 1 and 2.I've included the manpages patch at the end of this series. (Note that the manpage documents the behavior of the future glibc wrapper as well as the raw syscall.) Here's a formatted plain-text version of the manpage for reference:
CLONE4(2) Linux Programmer's Manual CLONE4(2)
NAME
clone4 - create a child processSYNOPSIS
/* Prototype for the glibc wrapper function */ #define _GNU_SOURCE #include <sched.h> int clone4(uint64_t flags, size_t args_size, struct clone4_args *args, int (*fn)(void *), void *arg); /* Prototype for the raw system call */ int clone4(unsigned flags_high, unsigned flags_low, unsigned long args_size, struct clone4_args *args); struct clone4_args { pid_t *ptid; pid_t *ctid; unsigned long stack_start; unsigned long stack_size; unsigned long tls; int *clonefd; unsigned clonefd_flags; };DESCRIPTION
clone4()creates a new process, similar to clone(2) and fork(2).clone4()supports additional flags that clone(2) does not, and accepts arguments via an extensible structure.args points to a
clone4_argsstructure, andargs_sizemust contain the
size of that structure, as understood by the caller. If the caller
passes a shorter structure than the kernel expects, the remaining fields will default to 0. If the caller passes a larger structure than the kernel expects (such as one from a newer kernel),clone4()will return EINVAL. Theclone4_argsstructure may gain additional fields at the end in the future, and callers must only pass a size that encom‐ passes the number of fields they understand. If the caller passes 0 forargs_size, args is ignored and may be NULL.In the
clone4_argsstructure, ptid, ctid,stack_start,stack_size, and tls have the same semantics as they do with clone(2) and clone2(2).In the glibc wrapper, fn and arg have the same semantics as they do with clone(2). As with clone(2), the underlying system call works more like fork(2), returning 0 in the child process; the glibc wrapper sim‐ plifies thread execution by calling fn(arg) and exiting the child when that function exits.
The 64-bit flags argument (split into the 32-bit
flags_highandflags_lowarguments in the kernel interface for portability across architectures) accepts all the same flags as clone(2), with the excep‐ tion of the obsoleteCLONE_PID,CLONE_DETACHED, andCLONE_STOPPED. In addition, flags accepts the following flags:
CLONE_AUTOREAP
When the new process exits, immediately reap it, rather than keeping it around as a "zombie" until a call to waitpid(2) or similar. Without this flag, the kernel will automatically reap a process if its exit signal is set to SIGCHLD, and if the par‐ ent process has SIGCHLD set toSIG_IGNor has a SIGCHLD handler installed withSA_NOCLDWAIT(see sigaction(2)).CLONE_AUTOREAPallows the calling process to enable automatic reaping with an exit signal other than SIGCHLD (including 0 to disable the exit signal), and does not depend on the configuration of process- wide signal handling.
CLONE_FD
Return a file descriptor associated with the new process, stor‐
ing it in location clonefd in the parent's address space. When
the new process exits, the file descriptor will become available for reading.Unlike using signalfd(2) for the SIGCHLD signal, the file descriptor returned by
clone4()with theCLONE_FDflag works even with SIGCHLD unblocked in one or more threads of the parent process, allowing the process to have different handlers for different child processes, such as those created by a library, without introducing race conditions around process-wide signal handling.
Quoted 8 lines
clonefd_flagsmay contain the following additional flags for use withCLONE_FD:
O_CLOEXEC
Set the close-on-exec flag on the new file descriptor. See the description of theO_CLOEXECflag in open(2) for reasons why this may be useful.
This begs the question: what happens when all CLONE_FD fds for a process are closed? Will the parent get SIGCHLD instead, will it auto-reap, or will it be un-wait-able (I assume not this...)
Quoted 141 lines
O_NONBLOCK
Set theO_NONBLOCKflag on the new file descriptor. Using this flag saves extra calls to fcntl(2) to achieve the same result.The returned file descriptor supports the following operations:
read(2) (and similar)
When the new process exits, reading from the file descriptor produces a singleclonefd_infostructure:struct
clonefd_info{
uint32_tcode; /* Signal code */
uint32_tstatus; /* Exit status or signal */uint64_tutime; /* User CPU time */uint64_tstime; /* System CPU time */
};If the new process has not yet exited, read(2) either blocks until it does, or fails with the error EAGAIN if the file descriptor has
O_NONBLOCKset.Future kernels may extend
clonefd_infoby appending addi‐ tional fields to the end. Callers should read as many bytes as they understand; unread data will be discarded, and subsequent reads after the first will return 0 to indicate end-of-file. Callers requesting more bytes than the kernel provides (such as callers expecting a newerclonefd_infostructure) will receive a shorter structure from older kernels.poll(2), select(2), epoll(7) (and similar)
The file descriptor is readable (the select(2) readfds argument; the poll(2) POLLIN flag) if the new process has exited.close(2)
When the file descriptor is no longer required it should be closed.C library/kernel ABI differences
As with clone(2), the rawclone4()system call corresponds more closely to fork(2) in that execution in the child continues from the point of the call.Unlike clone(2), the raw system call interface for
clone4()accepts arguments in the same order on all architectures.The raw system call accepts flags as two 32-bit arguments,
flags_highandflags_low, to simplify portability across 32-bit and 64-bit archi‐ tectures and calling conventions. The glibc wrapper accepts flags as a single 64-bit argument for convenience.RETURN VALUE
For the glibc wrapper, on success,clone4()returns the new process ID to the calling process, and the new process begins running at the spec‐ ified function.For the raw syscall, on success,
clone4()returns the new process ID to the calling process, and returns 0 in the new process.On failure,
clone4()returns -1 and sets errno accordingly.ERRORS
clone4()can return any error from clone(2), as well as the following additional errors:EFAULT args is outside your accessible address space.
EINVAL flags contained an unknown flag.
EINVAL flags included
CLONE_FDandclonefd_flagscontained an unknown
flag.EINVAL flags included
CLONE_FD, but the kernel configuration does not
have theCONFIG_CLONEFDoption enabled.EMFILE flags included
CLONE_FD, but the new file descriptor would
exceed the process limit on open file descriptors.ENFILE flags included
CLONE_FD, but the new file descriptor would
exceed the system-wide limit on open file descriptors.ENODEV flags included
CLONE_FD, butclone4()could not mount the
(internal) anonymous inode device.CONFORMING TO
clone4()is Linux-specific and should not be used in programs intended to be portable.SEE ALSO
clone(2), epoll(7), poll(2), pthreads(7), read(2), select(2)Linux 2015-03-14 CLONE4(2)
Josh Triplett and Thiago Macieira (7):
clone: Support passing tls argument via C rather thanpt_regsmagic
x86: Opt intoHAVE_COPY_THREAD_TLS, for both 32-bit and 64-bit Introduce a new clone4 syscall with more flag bits and extensible argumentskernel/fork.c: Pass arguments to_do_forkandcopy_processusingclone4_args
clone4: Add aCLONE_AUTOREAPflag to automatically reap the child process
signal: Factor out a helper function to processtask_structexit_code
clone4: Add aCLONE_FDflag to get task exit notification via fdarch/Kconfig | 7 ++ arch/x86/Kconfig | 1 + arch/x86/ia32/ia32entry.S | 3 +- arch/x86/kernel/entry_64.S | 1 + arch/x86/kernel/process_32.c | 6 +- arch/x86/kernel/process_64.c | 8 +-- arch/x86/syscalls/syscall_32.tbl | 1 + arch/x86/syscalls/syscall_64.tbl | 2 + include/linux/compat.h | 14 ++++ include/linux/sched.h | 22 ++++++ include/linux/syscalls.h | 6 +- include/uapi/asm-generic/unistd.h | 4 +- include/uapi/linux/sched.h | 55 ++++++++++++++- init/Kconfig | 21 ++++++ kernel/Makefile | 1 + kernel/clonefd.c | 121 ++++++++++++++++++++++++++++++++ kernel/clonefd.h | 32 +++++++++ kernel/exit.c | 4 ++ kernel/fork.c | 142 ++++++++++++++++++++++++++++++-------- kernel/signal.c | 26 ++++--- kernel/sys_ni.c | 1 + 21 files changed, 426 insertions(+), 52 deletions(-) create mode 100644 kernel/clonefd.c create mode 100644 kernel/clonefd.h
Quoted 3 lines
--
2.1.4
Looks promising!
-Kees
Signature · 6 lines
Kees Cook Chrome OS Security -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 8 lines · On Monday 16 March 2015 14:44:20 Kees Cook wrote:
Quoted 4 lines
O_CLOEXEC
Set the close-on-exec flag on the new file
descriptor. See the description of theO_CLOEXECflag in open(2) for reasons why this may be useful.This begs the question: what happens when all
CLONE_FDfds for a process are closed? Will the parent get SIGCHLD instead, will it auto-reap, or will it be un-wait-able (I assume not this...)
Depends on CLONE_AUTOREAP. If it's on, then no one gets SIGCHLD, no one can wait() on it and the process autoreaps itself.
If it's no active, then the old rules apply: parent gets SIGCHILD and can wait(). If the parent exited first, then the child gets reparented to init, which can do the wait().
A child without CLONE_AUTOREAP should be wait()able. If it gets wait()ed before the clonefd is read, the clonefd() will return a 0 read. If it gets read before wait, then wait() reaps another child or returns -ECHILD. That's no different than two threads doing simultaneous wait() on the same child.
Signature · 2 lines
Thiago Macieira - thiago.macieira (AT) intel.com Software Architect - Intel Open Source Technology Center
Quoted 13 lines · Thiago Macieira
Quoted 8 lines · On Monday 16 March 2015 14:44:20 Kees Cook wrote:
Quoted 4 lines
O_CLOEXEC
Set the close-on-exec flag on the new file
descriptor. See the description of theO_CLOEXECflag in open(2) for reasons why this may be useful.This begs the question: what happens when all
CLONE_FDfds for a process are closed? Will the parent get SIGCHLD instead, will it auto-reap, or will it be un-wait-able (I assume not this...)Depends on
CLONE_AUTOREAP. If it's on, then no one gets SIGCHLD, no one canwait()on it and the process autoreaps itself.
Quoted 8 lines
If it's no active, then the old rules apply: parent gets SIGCHILD and can
wait(). If the parent exited first, then the child gets reparented to init, which can do thewait().A child without
CLONE_AUTOREAPshould be wait()able. If it gets wait()ed before the clonefd is read, theclonefd()will return a 0 read. If it gets read before wait, thenwait()reaps another child or returns-ECHILD. That's no different than two threads doing simultaneouswait()on the same child.
Cool. I think detailing this in the manpage would be helpful.
And just so I understand the races here, what happens in CLONE_FD (without CLONE_AUTOREAP) case where the child dies, but the parent never reads from the CLONE_FD fd, and closes it (or dies)? Will the modes switch that late in the child's lifetime? (i.e. even though the details were written to the fd, they were never read, yet it'll still switch and generate a SIGCHLD, etc?)
Thanks!
-Kees
Signature · 2 lines
Kees Cook Chrome OS Security
Quoted 6 lines · On Monday 16 March 2015 15:36:16 Kees Cook wrote:
And just so I understand the races here, what happens in
CLONE_FD(withoutCLONE_AUTOREAP) case where the child dies, but the parent never reads from theCLONE_FDfd, and closes it (or dies)? Will the modes switch that late in the child's lifetime? (i.e. even though the details were written to the fd, they were never read, yet it'll still switch and generate a SIGCHLD, etc?)
What happens to a child that dies during the parent's lifetime but the parent exits without reaping the child?
The same should happen, whatever that behaviour is.
Signature · 2 lines
Thiago Macieira - thiago.macieira (AT) intel.com Software Architect - Intel Open Source Technology Center
Quoted 8 lines · Thiago Macieira
Quoted 6 lines · On Monday 16 March 2015 15:36:16 Kees Cook wrote:
And just so I understand the races here, what happens in
CLONE_FD(withoutCLONE_AUTOREAP) case where the child dies, but the parent never reads from theCLONE_FDfd, and closes it (or dies)? Will the modes switch that late in the child's lifetime? (i.e. even though the details were written to the fd, they were never read, yet it'll still switch and generate a SIGCHLD, etc?)
Quoted 4 lines
What happens to a child that dies during the parent's lifetime but the parent exits without reaping the child?
The same should happen, whatever that behaviour is.
Okay, sounds like the waitpid internals are tied to the read, not the child death. Perfect, that means it'll still be waitpid-able by whatever process inherits the zombie. Sounds good! Thanks for clarifying. :)
-Kees
Signature · 2 lines
Kees Cook Chrome OS Security
Quoted 24 lines · Kees Cook
Quoted 13 lines · Thiago Macieira
Quoted 8 lines · On Monday 16 March 2015 14:44:20 Kees Cook wrote:
Quoted 4 lines
O_CLOEXEC
Set the close-on-exec flag on the new file
descriptor. See the description of theO_CLOEXECflag in open(2) for reasons why this may be useful.This begs the question: what happens when all
CLONE_FDfds for a process are closed? Will the parent get SIGCHLD instead, will it auto-reap, or will it be un-wait-able (I assume not this...)Depends on
CLONE_AUTOREAP. If it's on, then no one gets SIGCHLD, no one canwait()on it and the process autoreaps itself.Quoted 8 lines
If it's no active, then the old rules apply: parent gets SIGCHILD and can
wait(). If the parent exited first, then the child gets reparented to init, which can do thewait().A child without
CLONE_AUTOREAPshould be wait()able. If it gets wait()ed before the clonefd is read, theclonefd()will return a 0 read. If it gets read before wait, thenwait()reaps another child or returns-ECHILD. That's no different than two threads doing simultaneouswait()on the same child.
Quoted 8 lines
Cool. I think detailing this in the manpage would be helpful.
And just so I understand the races here, what happens in
CLONE_FD(withoutCLONE_AUTOREAP) case where the child dies, but the parent never reads from theCLONE_FDfd, and closes it (or dies)? Will the modes switch that late in the child's lifetime? (i.e. even though the details were written to the fd, they were never read, yet it'll still switch and generate a SIGCHLD, etc?)
This doesn't actually work like a pipe; the details aren't "written" to the fd. The data is generated at read time, and if you never read, that's fine. There's no semantic meaning attached to reading from the clonefd; you still have to wait on the process if you don't pass CLONE_AUTOREAP. (Or you can block SIGCHLD or use SA_NOCLDWAIT, if you control the calling process's signal handling; AUTOREAP just lets you avoid interacting with the calling process's signal handling.)
See my previous response for the rest.
- Josh Triplett
Quoted 6 lines · Thiago Macieira
Quoted 5 lines · On Monday 16 March 2015 14:44:20 Kees Cook wrote:
Quoted 4 lines
O_CLOEXEC
Set the close-on-exec flag on the new file
descriptor. See the description of theO_CLOEXECflag in open(2) for reasons why this may be useful.
Quoted 6 lines
Quoted 3 lines
This begs the question: what happens when all
CLONE_FDfds for a process are closed? Will the parent get SIGCHLD instead, will it auto-reap, or will it be un-wait-able (I assume not this...)Depends on
CLONE_AUTOREAP. If it's on, then no one gets SIGCHLD, no one canwait()on it and the process autoreaps itself.
Minor nit: CLONE_AUTOREAP makes the process autoreap and nobody can wait on it, but if you pass SIGCHLD or some other exit signal to clone then you'll still get that signal.
Quoted 3 lines
If it's no active, then the old rules apply: parent gets SIGCHILD and can
wait(). If the parent exited first, then the child gets reparented to init, which can do thewait().
Right.
Quoted 4 lines
A child without
CLONE_AUTOREAPshould be wait()able. If it gets wait()ed before the clonefd is read, theclonefd()will return a 0 read. If it gets read before wait, thenwait()reaps another child or returns-ECHILD. That's no different than two threads doing simultaneouswait()on the same child.
Hrm? That isn't the semantics we implemented; you'll *always* get an exit notification via the clonefd if you have it open, with or without autoreap and whether or not a wait has occurred yet. And reading from the clonefd does not serve as a wait; if you don't pass CLONE_AUTOREAP, you'll still need to wait on the process.
- Josh Triplett
Quoted 11 lines · On Monday 16 March 2015 16:29:49 [email protected]…
Quoted 6 lines
A child without
CLONE_AUTOREAPshould be wait()able. If it gets wait()ed before the clonefd is read, theclonefd()will return a 0 read. If it gets
read before wait, thenwait()reaps another child or returns-ECHILD. That's no different than two threads doing simultaneouswait()on the same child.Hrm? That isn't the semantics we implemented; you'll *always* get an exit notification via the clonefd if you have it open, with or without autoreap and whether or not a wait has occurred yet. And reading from the clonefd does not serve as a wait; if you don't pass
CLONE_AUTOREAP, you'll still need to wait on the process.
Ah, I see what you're saying. Ok, I stand corrected: a child without CLONE_AUTOREAP must be wait()ed on and whoever waits on it will get information. In addition to that, the information is available on the clonefd and it can happen at any time, before or after the wait().
In the case of an orphaned child, the file descriptor will close, that's all. No modification is necessary to init.
Signature · 2 lines
Thiago Macieira - thiago.macieira (AT) intel.com Software Architect - Intel Open Source Technology Center
Quoted 11 lines · On Mon, Mar 16, 2015 at 11:29 PM, <[email protected]> wrote:
Quoted 10 lines · Thiago Macieira
Quoted 8 lines · On Monday 16 March 2015 14:44:20 Kees Cook wrote:
Quoted 4 lines
O_CLOEXEC
Set the close-on-exec flag on the new file
descriptor. See the description of theO_CLOEXECflag in open(2) for reasons why this may be useful.This begs the question: what happens when all
CLONE_FDfds for a process are closed? Will the parent get SIGCHLD instead, will it auto-reap, or will it be un-wait-able (I assume not this...)
Quoted 6 lines
Quoted 2 lines
Depends on
CLONE_AUTOREAP. If it's on, then no one gets SIGCHLD, no one canwait()on it and the process autoreaps itself.Minor nit:
CLONE_AUTOREAPmakes the process autoreap and nobody can wait on it, but if you pass SIGCHLD or some other exit signal to clone then you'll still get that signal.
Quick query: does CLONE_AUTOREAP also affect waiting for non-exit events (i.e. WUNTRACED / WCONTINUED), by original parent and/or ptracer?
Quoted 15 lines · David Drysdale
Quoted 14 lines · On Mon, Mar 16, 2015 at 11:29 PM, <[email protected]> wrote:
Quoted 12 lines · Thiago Macieira
Quoted 8 lines · On Monday 16 March 2015 14:44:20 Kees Cook wrote:
Quoted 4 lines
O_CLOEXEC
Set the close-on-exec flag on the new file
descriptor. See the description of theO_CLOEXECflag in open(2) for reasons why this may be useful.This begs the question: what happens when all
CLONE_FDfds for a process are closed? Will the parent get SIGCHLD instead, will it auto-reap, or will it be un-wait-able (I assume not this...)Depends on
CLONE_AUTOREAP. If it's on, then no one gets SIGCHLD, no one canwait()on it and the process autoreaps itself.
Quoted 6 lines
Quoted 3 lines
Minor nit:
CLONE_AUTOREAPmakes the process autoreap and nobody can wait on it, but if you pass SIGCHLD or some other exit signal to clone then you'll still get that signal.Quick query: does
CLONE_AUTOREAPalso affect waiting for non-exit events (i.e. WUNTRACED / WCONTINUED), by original parent and/or ptracer?
It shouldn't, no. You can't wait on the process to exit (you'll get -ECHLD after it wakes up), but you can wait on it to continue or similar; none of the autoreap changes should affect that.
- Josh Triplett
Quoted 7 lines · Kees Cook
Quoted 3 lines · Josh Triplett
- Make poll on a
CLONE_FDfor an exited task also return POLLHUP, for compatibility with FreeBSD's pdfork. Thanks to David Drysdale for calling attention to pdfork.I think POLLHUP should be mentioned in the manpage (now it only mentions POLLIN).
Added for v3.
Quoted 18 lines
Quoted 18 lines
CLONE_FD
Return a file descriptor associated with the new process, stor‐
ing it in location clonefd in the parent's address space. When
the new process exits, the file descriptor will become available for reading.Unlike using signalfd(2) for the SIGCHLD signal, the file descriptor returned by
clone4()with theCLONE_FDflag works even with SIGCHLD unblocked in one or more threads of the parent process, allowing the process to have different handlers for different child processes, such as those created by a library, without introducing race conditions around process-wide signal handling.
clonefd_flagsmay contain the following additional flags for use withCLONE_FD:
Quoted 8 lines
Quoted 4 lines
O_CLOEXEC
Set the close-on-exec flag on the new file descriptor. See the description of theO_CLOEXECflag in open(2) for reasons why this may be useful.This begs the question: what happens when all
CLONE_FDfds for a process are closed? Will the parent get SIGCHLD instead, will it auto-reap, or will it be un-wait-able (I assume not this...)
Whether the parent gets SIGCHLD is determined only by what signal you request in clone; if you clone with CLONE_FD | SIGCHLD (or CLONE_AUTOREAP | CLONE_FD | SIGCHLD), you'll get notification via both clonefd (if you have one) and signal (if you have a handler). If you pass a 0 signal (just CLONE_FD or CLONE_AUTOREAP | CLONE_FD), you'll receive no signal, only the notification via clonefd. Independently, if you have CLONE_AUTOREAP set, the process will autoreap.
Those are all orthogonal now.
If you close the clonefd, nothing special happens other than a put_task_struct. While this is conceptually somewhat like a pipe, the data is actually generated at read time, so the task exit doesn't care whether there's a live clonefd or not. (Or, in the future, if there are multiple live clonefds for the same process.)
Quoted 1 line
Looks promising!
Thanks!
And thanks for catching the manpage issue. I'd definitely welcome any comments you have on the implementation as well.
- Josh Triplett
So I finally got around to having a look at this, and one thing caught my eye:
Quoted 10 lines
read(2) (and similar)
When the new process exits, reading from the file descriptor produces a singleclonefd_infostructure:struct
clonefd_info{
uint32_tcode; /* Signal code */
uint32_tstatus; /* Exit status or signal */uint64_tutime; /* User CPU time */uint64_tstime; /* System CPU time */
};
This would appear to assume that a clonefd_info structure is the only thing that will ever be read from this descriptor. It seems to me that there is the potential for, someday, wanting to be able to read and write other things as well. Should this structure be marked with type and length fields so that other structures could be added in the future?
(I suppose we could just use ioctl() for any other functionality in the future, though...:)
jon
Quoted 14 lines · Jonathan Corbet
So I finally got around to having a look at this, and one thing caught my eye:
Quoted 10 lines
read(2) (and similar)
When the new process exits, reading from the file descriptor produces a singleclonefd_infostructure:struct
clonefd_info{
uint32_tcode; /* Signal code */
uint32_tstatus; /* Exit status or signal */uint64_tutime; /* User CPU time */uint64_tstime; /* System CPU time */
};
Quoted 5 lines
This would appear to assume that a
clonefd_infostructure is the only thing that will ever be read from this descriptor. It seems to me that there is the potential for, someday, wanting to be able to read and write other things as well. Should this structure be marked with type and length fields so that other structures could be added in the future?
I don't think it makes sense for a caller to get an arbitrary structure on read(), and have to figure out what they got and ignore something they don't understand. Instead, I think it makes more sense for the caller to say "Hey, here's a flag saying I understand the new thing, go ahead and give me the new thing". So, for instance, if you want to receive SIGSTOP/SIGCONT messages for child processes through this descriptor, we could add a flag for that.
- Josh Triplett
Quoted 6 lines · On Tue, 31 Mar 2015 15:02:24 -0700 [email protected] wrote:
Quoted 5 lines
This would appear to assume that a
clonefd_infostructure is the only thing that will ever be read from this descriptor. It seems to me that there is the potential for, someday, wanting to be able to read and write other things as well. Should this structure be marked with type and length fields so that other structures could be added in the future?
Quoted 7 lines
I don't think it makes sense for a caller to get an arbitrary structure on
read(), and have to figure out what they got and ignore something they don't understand. Instead, I think it makes more sense for the caller to say "Hey, here's a flag saying I understand the new thing, go ahead and give me the new thing". So, for instance, if you want to receive SIGSTOP/SIGCONT messages for child processes through this descriptor, we could add a flag for that.
The flag is fine, but, once we have set that flag saying we want those messages, how do we know which type of structure we've gotten? That's the piece of the puzzle I'm missing, sorry if I'm being overly slow.
Thanks,
jon
Quoted 17 lines · Jonathan Corbet
Quoted 6 lines · On Tue, 31 Mar 2015 15:02:24 -0700 [email protected]…
Quoted 5 lines
This would appear to assume that a
clonefd_infostructure is the only thing that will ever be read from this descriptor. It seems to me that there is the potential for, someday, wanting to be able to read and write other things as well. Should this structure be marked with type and length fields so that other structures could be added in the future?Quoted 7 lines
I don't think it makes sense for a caller to get an arbitrary structure on
read(), and have to figure out what they got and ignore something they don't understand. Instead, I think it makes more sense for the caller to say "Hey, here's a flag saying I understand the new thing, go ahead and give me the new thing". So, for instance, if you want to receive SIGSTOP/SIGCONT messages for child processes through this descriptor, we could add a flag for that.
Quoted 3 lines
The flag is fine, but, once we have set that flag saying we want those messages, how do we know which type of structure we've gotten? That's the piece of the puzzle I'm missing, sorry if I'm being overly slow.
If you pass a flag saying you can handle a new set of potential structures, those structures can then include any necessary disambiguating flags/IDs/etc. No need for them to match the current clonefd_info structure if userspace has opted into a new version.
- Josh Triplett
Quoted 9 lines · Josh Triplett
This patch series introduces a new clone flag,
CLONE_FD, which lets the caller receive child process exit notification via a file descriptor rather than SIGCHLD.CLONE_FDmakes it possible for libraries to safely launch and manage child processes on behalf of their caller, *without* taking over process-wide SIGCHLD handling (either via signal handler or signalfd).Note that signalfd for SIGCHLD does not suffice here, because that still receives notification for all child processes, and interferes with process-wide signal handling.
It has been suggested (e.g.,
<https://sourceware.org/bugzilla/show_bug.cgi?id=15661#c3>) that you can use the existing clone(2) without specifying SIGCHLD to create a new process. The resulting child process is not supposed to show up in wait(2), only in a waitpid(2) (or similar) explicitly specifying the PID. Is this not the case?
Signature · 1 line
Florian Weimer / Red Hat Product Security
Quoted 12 lines · On Friday 29 May 2015 09:43:35 Florian Weimer wrote:
Quoted 10 lines · Josh Triplett
This patch series introduces a new clone flag,
CLONE_FD, which lets the caller receive child process exit notification via a file descriptor rather than SIGCHLD.CLONE_FDmakes it possible for libraries to safely launch and manage child processes on behalf of their caller, *without* taking over process-wide SIGCHLD handling (either via signal handler or signalfd).Note that signalfd for SIGCHLD does not suffice here, because that still receives notification for all child processes, and interferes with process-wide signal handling.
Quoted 6 lines
It has been suggested (e.g.,
<https://sourceware.org/bugzilla/show_bug.cgi?id=15661#c3>) that you can use the existing clone(2) without specifying SIGCHLD to create a new process. The resulting child process is not supposed to show up in wait(2), only in a waitpid(2) (or similar) explicitly specifying the PID. Is this not the case?
Hi Florian
That sounds orthogonal to what we're looking for. Our objective is to get notification of when the child exited without resorting to SIGCHLD. If we use the regular clone(2) without SIGCHLD and without CLONE_FD, we get no notification. The only way to know of the child's termination is by a blocking waitpid(2), like you indicated, which is counter productive to our needs.
We need something we can select(2)/poll(2) on.
Signature · 2 lines
Thiago Macieira - thiago.macieira (AT) intel.com Software Architect - Intel Open Source Technology Center
Quoted 9 lines · Thiago Macieira
Quoted 6 lines
It has been suggested (e.g.,
<https://sourceware.org/bugzilla/show_bug.cgi?id=15661#c3>) that you can use the existing clone(2) without specifying SIGCHLD to create a new process. The resulting child process is not supposed to show up in wait(2), only in a waitpid(2) (or similar) explicitly specifying the PID. Is this not the case?Hi Florian
Quoted 7 lines
That sounds orthogonal to what we're looking for. Our objective is to get notification of when the child exited without resorting to SIGCHLD. If we use the regular clone(2) without SIGCHLD and without
CLONE_FD, we get no notification. The only way to know of the child's termination is by a blocking waitpid(2), like you indicated, which is counter productive to our needs.We need something we can select(2)/poll(2) on.
Thanks for the clarification. I agree that this is a separate and quite sensible use case.
Signature · 1 line
Florian Weimer / Red Hat Product Security