From f08d0c3a71114bb36d1722506d926bd497182781 Mon Sep 17 00:00:00 2001 From: Lorenzo Stoakes Date: Thu, 30 Jan 2025 20:40:26 +0000 Subject: [PATCH 01/25] pidfd: add PIDFD_SELF* sentinels to refer to own thread/process It is useful to be able to utilise the pidfd mechanism to reference the current thread or process (from a userland point of view - thread group leader from the kernel's point of view). Therefore introduce PIDFD_SELF_THREAD to refer to the current thread, and PIDFD_SELF_THREAD_GROUP to refer to the current thread group leader. For convenience and to avoid confusion from userland's perspective we alias these: * PIDFD_SELF is an alias for PIDFD_SELF_THREAD - This is nearly always what the user will want to use, as they would find it surprising if for instance fd's were unshared()'d and they wanted to invoke pidfd_getfd() and that failed. * PIDFD_SELF_PROCESS is an alias for PIDFD_SELF_THREAD_GROUP - Most users have no concept of thread groups or what a thread group leader is, and from userland's perspective and nomenclature this is what userland considers to be a process. We adjust pidfd_get_task() and the pidfd_send_signal() system call with specific handling for this, implementing this functionality for process_madvise(), process_mrelease() (albeit, using it here wouldn't really make sense) and pidfd_send_signal(). Signed-off-by: Lorenzo Stoakes Link: https://lore.kernel.org/r/24315a16a3d01a548dd45c7515f7d51c767e954e.1738268370.git.lorenzo.stoakes@oracle.com Signed-off-by: Christian Brauner --- include/uapi/linux/pidfd.h | 24 ++++++++ kernel/pid.c | 24 ++++++-- kernel/signal.c | 117 ++++++++++++++++++++++--------------- 3 files changed, 112 insertions(+), 53 deletions(-) diff --git a/include/uapi/linux/pidfd.h b/include/uapi/linux/pidfd.h index 4540f6301b8c..e0abd0b18841 100644 --- a/include/uapi/linux/pidfd.h +++ b/include/uapi/linux/pidfd.h @@ -23,6 +23,30 @@ #define PIDFD_INFO_SIZE_VER0 64 /* sizeof first published struct */ +/* + * The concept of process and threads in userland and the kernel is a confusing + * one - within the kernel every thread is a 'task' with its own individual PID, + * however from userland's point of view threads are grouped by a single PID, + * which is that of the 'thread group leader', typically the first thread + * spawned. + * + * To cut the Gideon knot, for internal kernel usage, we refer to + * PIDFD_SELF_THREAD to refer to the current thread (or task from a kernel + * perspective), and PIDFD_SELF_THREAD_GROUP to refer to the current thread + * group leader... + */ +#define PIDFD_SELF_THREAD -10000 /* Current thread. */ +#define PIDFD_SELF_THREAD_GROUP -20000 /* Current thread group leader. */ + +/* + * ...and for userland we make life simpler - PIDFD_SELF refers to the current + * thread, PIDFD_SELF_PROCESS refers to the process thread group leader. + * + * For nearly all practical uses, a user will want to use PIDFD_SELF. + */ +#define PIDFD_SELF PIDFD_SELF_THREAD +#define PIDFD_SELF_PROCESS PIDFD_SELF_THREAD_GROUP + struct pidfd_info { /* * This mask is similar to the request_mask in statx(2). diff --git a/kernel/pid.c b/kernel/pid.c index 924084713be8..22f5d2b2e290 100644 --- a/kernel/pid.c +++ b/kernel/pid.c @@ -564,15 +564,29 @@ struct pid *pidfd_get_pid(unsigned int fd, unsigned int *flags) */ struct task_struct *pidfd_get_task(int pidfd, unsigned int *flags) { - unsigned int f_flags; + unsigned int f_flags = 0; struct pid *pid; struct task_struct *task; + enum pid_type type; - pid = pidfd_get_pid(pidfd, &f_flags); - if (IS_ERR(pid)) - return ERR_CAST(pid); + switch (pidfd) { + case PIDFD_SELF_THREAD: + type = PIDTYPE_PID; + pid = get_task_pid(current, type); + break; + case PIDFD_SELF_THREAD_GROUP: + type = PIDTYPE_TGID; + pid = get_task_pid(current, type); + break; + default: + pid = pidfd_get_pid(pidfd, &f_flags); + if (IS_ERR(pid)) + return ERR_CAST(pid); + type = PIDTYPE_TGID; + break; + } - task = get_pid_task(pid, PIDTYPE_TGID); + task = get_pid_task(pid, type); put_pid(pid); if (!task) return ERR_PTR(-ESRCH); diff --git a/kernel/signal.c b/kernel/signal.c index 875e97f6205a..081f19a24506 100644 --- a/kernel/signal.c +++ b/kernel/signal.c @@ -4009,56 +4009,12 @@ static struct pid *pidfd_to_pid(const struct file *file) (PIDFD_SIGNAL_THREAD | PIDFD_SIGNAL_THREAD_GROUP | \ PIDFD_SIGNAL_PROCESS_GROUP) -/** - * sys_pidfd_send_signal - Signal a process through a pidfd - * @pidfd: file descriptor of the process - * @sig: signal to send - * @info: signal info - * @flags: future flags - * - * Send the signal to the thread group or to the individual thread depending - * on PIDFD_THREAD. - * In the future extension to @flags may be used to override the default scope - * of @pidfd. - * - * Return: 0 on success, negative errno on failure - */ -SYSCALL_DEFINE4(pidfd_send_signal, int, pidfd, int, sig, - siginfo_t __user *, info, unsigned int, flags) +static int do_pidfd_send_signal(struct pid *pid, int sig, enum pid_type type, + siginfo_t __user *info, unsigned int flags) { - int ret; - struct pid *pid; kernel_siginfo_t kinfo; - enum pid_type type; - - /* Enforce flags be set to 0 until we add an extension. */ - if (flags & ~PIDFD_SEND_SIGNAL_FLAGS) - return -EINVAL; - - /* Ensure that only a single signal scope determining flag is set. */ - if (hweight32(flags & PIDFD_SEND_SIGNAL_FLAGS) > 1) - return -EINVAL; - - CLASS(fd, f)(pidfd); - if (fd_empty(f)) - return -EBADF; - - /* Is this a pidfd? */ - pid = pidfd_to_pid(fd_file(f)); - if (IS_ERR(pid)) - return PTR_ERR(pid); - - if (!access_pidfd_pidns(pid)) - return -EINVAL; switch (flags) { - case 0: - /* Infer scope from the type of pidfd. */ - if (fd_file(f)->f_flags & PIDFD_THREAD) - type = PIDTYPE_PID; - else - type = PIDTYPE_TGID; - break; case PIDFD_SIGNAL_THREAD: type = PIDTYPE_PID; break; @@ -4071,6 +4027,8 @@ SYSCALL_DEFINE4(pidfd_send_signal, int, pidfd, int, sig, } if (info) { + int ret; + ret = copy_siginfo_from_user_any(&kinfo, info); if (unlikely(ret)) return ret; @@ -4088,8 +4046,71 @@ SYSCALL_DEFINE4(pidfd_send_signal, int, pidfd, int, sig, if (type == PIDTYPE_PGID) return kill_pgrp_info(sig, &kinfo, pid); - else - return kill_pid_info_type(sig, &kinfo, pid, type); + + return kill_pid_info_type(sig, &kinfo, pid, type); +} + +/** + * sys_pidfd_send_signal - Signal a process through a pidfd + * @pidfd: file descriptor of the process + * @sig: signal to send + * @info: signal info + * @flags: future flags + * + * Send the signal to the thread group or to the individual thread depending + * on PIDFD_THREAD. + * In the future extension to @flags may be used to override the default scope + * of @pidfd. + * + * Return: 0 on success, negative errno on failure + */ +SYSCALL_DEFINE4(pidfd_send_signal, int, pidfd, int, sig, + siginfo_t __user *, info, unsigned int, flags) +{ + struct pid *pid; + enum pid_type type; + + /* Enforce flags be set to 0 until we add an extension. */ + if (flags & ~PIDFD_SEND_SIGNAL_FLAGS) + return -EINVAL; + + /* Ensure that only a single signal scope determining flag is set. */ + if (hweight32(flags & PIDFD_SEND_SIGNAL_FLAGS) > 1) + return -EINVAL; + + switch (pidfd) { + case PIDFD_SELF_THREAD: + pid = get_task_pid(current, PIDTYPE_PID); + type = PIDTYPE_PID; + break; + case PIDFD_SELF_THREAD_GROUP: + pid = get_task_pid(current, PIDTYPE_TGID); + type = PIDTYPE_TGID; + break; + default: { + CLASS(fd, f)(pidfd); + if (fd_empty(f)) + return -EBADF; + + /* Is this a pidfd? */ + pid = pidfd_to_pid(fd_file(f)); + if (IS_ERR(pid)) + return PTR_ERR(pid); + + if (!access_pidfd_pidns(pid)) + return -EINVAL; + + /* Infer scope from the type of pidfd. */ + if (fd_file(f)->f_flags & PIDFD_THREAD) + type = PIDTYPE_PID; + else + type = PIDTYPE_TGID; + + return do_pidfd_send_signal(pid, sig, type, info, flags); + } + } + + return do_pidfd_send_signal(pid, sig, type, info, flags); } static int From e943271f7956e439e4c9ef3b7a13f7f00f6c9885 Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Wed, 5 Feb 2025 13:54:56 +0100 Subject: [PATCH 02/25] selftests/pidfd: add new PIDFD_SELF* defines They will be needed in selftests in follow-up patches. Signed-off-by: Christian Brauner --- tools/testing/selftests/pidfd/pidfd.h | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tools/testing/selftests/pidfd/pidfd.h b/tools/testing/selftests/pidfd/pidfd.h index 0b96ac4b8ce5..027ebaf14844 100644 --- a/tools/testing/selftests/pidfd/pidfd.h +++ b/tools/testing/selftests/pidfd/pidfd.h @@ -50,6 +50,22 @@ #define PIDFD_NONBLOCK O_NONBLOCK #endif +#ifndef PIDFD_SELF_THREAD +#define PIDFD_SELF_THREAD -10000 /* Current thread. */ +#endif + +#ifndef PIDFD_SELF_THREAD_GROUP +#define PIDFD_SELF_THREAD_GROUP -20000 /* Current thread group leader. */ +#endif + +#ifndef PIDFD_SELF +#define PIDFD_SELF PIDFD_SELF_THREAD +#endif + +#ifndef PIDFD_SELF_PROCESS +#define PIDFD_SELF_PROCESS PIDFD_SELF_THREAD_GROUP +#endif + /* * The kernel reserves 300 pids via RESERVED_PIDS in kernel/pid.c * That means, when it wraps around any pid < 300 will be skipped. From 2bbf47f2d396ee32068042a99ecf4da955ce88ec Mon Sep 17 00:00:00 2001 From: Lorenzo Stoakes Date: Thu, 30 Jan 2025 20:40:30 +0000 Subject: [PATCH 03/25] selftests/pidfd: add tests for PIDFD_SELF_* Add tests to assert that PIDFD_SELF* correctly refers to the current thread and process. We explicitly test pidfd_send_signal(), however We defer testing of mm-specific functionality which uses pidfd, namely process_madvise() and process_mrelease() to mm testing (though note the latter can not be sensibly tested as it would require the testing process to be dying). We also correct the pidfd_open_test.c fields which refer to .request_mask whereas the UAPI header refers to .mask, which otherwise break the import of the UAPI header. Signed-off-by: Lorenzo Stoakes Link: https://lore.kernel.org/r/7ab0e48b26ba53abf7b703df2dd11a2e99b8efb2.1738268370.git.lorenzo.stoakes@oracle.com Reviewed-by: Shakeel Butt Signed-off-by: Christian Brauner --- .../testing/selftests/pidfd/pidfd_open_test.c | 6 +- tools/testing/selftests/pidfd/pidfd_test.c | 80 +++++++++++++++---- 2 files changed, 69 insertions(+), 17 deletions(-) diff --git a/tools/testing/selftests/pidfd/pidfd_open_test.c b/tools/testing/selftests/pidfd/pidfd_open_test.c index ce413a221bac..9a40ccb1ff6d 100644 --- a/tools/testing/selftests/pidfd/pidfd_open_test.c +++ b/tools/testing/selftests/pidfd/pidfd_open_test.c @@ -31,7 +31,7 @@ #define PIDFD_INFO_CGROUPID (1UL << 0) struct pidfd_info { - __u64 request_mask; + __u64 mask; __u64 cgroupid; __u32 pid; __u32 tgid; @@ -148,7 +148,7 @@ out: int main(int argc, char **argv) { struct pidfd_info info = { - .request_mask = PIDFD_INFO_CGROUPID, + .mask = PIDFD_INFO_CGROUPID, }; int pidfd = -1, ret = 1; pid_t pid; @@ -227,7 +227,7 @@ int main(int argc, char **argv) getegid(), info.sgid); goto on_error; } - if ((info.request_mask & PIDFD_INFO_CGROUPID) && info.cgroupid == 0) { + if ((info.mask & PIDFD_INFO_CGROUPID) && info.cgroupid == 0) { ksft_print_msg("cgroupid should not be 0 when PIDFD_INFO_CGROUPID is set\n"); goto on_error; } diff --git a/tools/testing/selftests/pidfd/pidfd_test.c b/tools/testing/selftests/pidfd/pidfd_test.c index e9728e86b4f2..fcd85cad9f18 100644 --- a/tools/testing/selftests/pidfd/pidfd_test.c +++ b/tools/testing/selftests/pidfd/pidfd_test.c @@ -42,12 +42,41 @@ static pid_t pidfd_clone(int flags, int *pidfd, int (*fn)(void *)) #endif } -static int signal_received; +static pthread_t signal_received; static void set_signal_received_on_sigusr1(int sig) { if (sig == SIGUSR1) - signal_received = 1; + signal_received = pthread_self(); +} + +static int send_signal(int pidfd) +{ + int ret = 0; + + if (sys_pidfd_send_signal(pidfd, SIGUSR1, NULL, 0) < 0) { + ret = -EINVAL; + goto exit; + } + + if (signal_received != pthread_self()) { + ret = -EINVAL; + goto exit; + } + +exit: + signal_received = 0; + return ret; +} + +static void *send_signal_worker(void *arg) +{ + int pidfd = (int)(intptr_t)arg; + int ret; + + /* We forward any errors for the caller to handle. */ + ret = send_signal(pidfd); + return (void *)(intptr_t)ret; } /* @@ -56,8 +85,11 @@ static void set_signal_received_on_sigusr1(int sig) */ static int test_pidfd_send_signal_simple_success(void) { - int pidfd, ret; + int pidfd; const char *test_name = "pidfd_send_signal send SIGUSR1"; + pthread_t thread; + void *thread_res; + int err; if (!have_pidfd_send_signal) { ksft_test_result_skip( @@ -66,25 +98,45 @@ static int test_pidfd_send_signal_simple_success(void) return 0; } + signal(SIGUSR1, set_signal_received_on_sigusr1); + + /* Try sending a signal to ourselves via /proc/self. */ pidfd = open("/proc/self", O_DIRECTORY | O_CLOEXEC); if (pidfd < 0) ksft_exit_fail_msg( "%s test: Failed to open process file descriptor\n", test_name); - - signal(SIGUSR1, set_signal_received_on_sigusr1); - - ret = sys_pidfd_send_signal(pidfd, SIGUSR1, NULL, 0); + err = send_signal(pidfd); + if (err) + ksft_exit_fail_msg( + "%s test: Error %d on sending pidfd signal\n", + test_name, err); close(pidfd); - if (ret < 0) - ksft_exit_fail_msg("%s test: Failed to send signal\n", - test_name); - if (signal_received != 1) - ksft_exit_fail_msg("%s test: Failed to receive signal\n", - test_name); + /* Now try the same thing only using PIDFD_SELF_THREAD_GROUP. */ + err = send_signal(PIDFD_SELF_THREAD_GROUP); + if (err) + ksft_exit_fail_msg( + "%s test: Error %d on PIDFD_SELF_THREAD_GROUP signal\n", + test_name, err); + + /* + * Now try the same thing in a thread and assert thread ID is equal to + * worker thread ID. + */ + if (pthread_create(&thread, NULL, send_signal_worker, + (void *)(intptr_t)PIDFD_SELF_THREAD)) + ksft_exit_fail_msg("%s test: Failed to create thread\n", + test_name); + if (pthread_join(thread, &thread_res)) + ksft_exit_fail_msg("%s test: Failed to join thread\n", + test_name); + err = (int)(intptr_t)thread_res; + if (err) + ksft_exit_fail_msg( + "%s test: Error %d on PIDFD_SELF_THREAD signal\n", + test_name, err); - signal_received = 0; ksft_test_result_pass("%s test: Sent signal\n", test_name); return 0; } From 62d648c4429a5edcfcfae03da15d2268d66e1a78 Mon Sep 17 00:00:00 2001 From: Lorenzo Stoakes Date: Thu, 30 Jan 2025 20:40:31 +0000 Subject: [PATCH 04/25] selftests/mm: use PIDFD_SELF in guard pages test Now we have PIDFD_SELF available for process_madvise(), make use of it in the guard pages test. This is both more convenient and asserts that PIDFD_SELF works as expected. Signed-off-by: Lorenzo Stoakes Link: https://lore.kernel.org/r/69fbbe088d3424de9983e145228459cb05a8f13d.1738268370.git.lorenzo.stoakes@oracle.com Reviewed-by: Shakeel Butt Signed-off-by: Christian Brauner --- tools/testing/selftests/mm/guard-pages.c | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/tools/testing/selftests/mm/guard-pages.c b/tools/testing/selftests/mm/guard-pages.c index ece37212a8a2..525c50d3ec23 100644 --- a/tools/testing/selftests/mm/guard-pages.c +++ b/tools/testing/selftests/mm/guard-pages.c @@ -19,6 +19,8 @@ #include #include +#include "../pidfd/pidfd.h" + /* * Ignore the checkpatch warning, as per the C99 standard, section 7.14.1.1: * @@ -50,11 +52,6 @@ static void handle_fatal(int c) siglongjmp(signal_jmp_buf, c); } -static int pidfd_open(pid_t pid, unsigned int flags) -{ - return syscall(SYS_pidfd_open, pid, flags); -} - static ssize_t sys_process_madvise(int pidfd, const struct iovec *iovec, size_t n, int advice, unsigned int flags) { @@ -370,14 +367,10 @@ TEST_F(guard_pages, multi_vma) TEST_F(guard_pages, process_madvise) { const unsigned long page_size = self->page_size; - pid_t pid = getpid(); - int pidfd = pidfd_open(pid, 0); char *ptr_region, *ptr1, *ptr2, *ptr3; ssize_t count; struct iovec vec[6]; - ASSERT_NE(pidfd, -1); - /* Reserve region to map over. */ ptr_region = mmap(NULL, 100 * page_size, PROT_NONE, MAP_ANON | MAP_PRIVATE, -1, 0); @@ -425,7 +418,7 @@ TEST_F(guard_pages, process_madvise) ASSERT_EQ(munmap(&ptr_region[99 * page_size], page_size), 0); /* Now guard in one step. */ - count = sys_process_madvise(pidfd, vec, 6, MADV_GUARD_INSTALL, 0); + count = sys_process_madvise(PIDFD_SELF, vec, 6, MADV_GUARD_INSTALL, 0); /* OK we don't have permission to do this, skip. */ if (count == -1 && errno == EPERM) @@ -446,7 +439,7 @@ TEST_F(guard_pages, process_madvise) ASSERT_FALSE(try_read_write_buf(&ptr3[19 * page_size])); /* Now do the same with unguard... */ - count = sys_process_madvise(pidfd, vec, 6, MADV_GUARD_REMOVE, 0); + count = sys_process_madvise(PIDFD_SELF, vec, 6, MADV_GUARD_REMOVE, 0); /* ...and everything should now succeed. */ @@ -463,7 +456,6 @@ TEST_F(guard_pages, process_madvise) ASSERT_EQ(munmap(ptr1, 10 * page_size), 0); ASSERT_EQ(munmap(ptr2, 5 * page_size), 0); ASSERT_EQ(munmap(ptr3, 20 * page_size), 0); - close(pidfd); } /* Assert that unmapping ranges does not leave guard markers behind. */ From 816b2e602035214de9ca2f68751e64ecef023dd1 Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Wed, 5 Mar 2025 11:08:11 +0100 Subject: [PATCH 05/25] pidfs: switch to copy_struct_to_user() We have a helper that deals with all the required logic. Link: https://lore.kernel.org/r/20250305-work-pidfs-kill_on_last_close-v3-1-c8c3d8361705@kernel.org Reviewed-by: Jeff Layton Reviewed-by: Oleg Nesterov Signed-off-by: Christian Brauner --- fs/pidfs.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/fs/pidfs.c b/fs/pidfs.c index 049352f973de..aa8c8bda8c8f 100644 --- a/fs/pidfs.c +++ b/fs/pidfs.c @@ -276,10 +276,7 @@ static long pidfd_info(struct task_struct *task, unsigned int cmd, unsigned long * userspace knows about will be copied. If userspace provides a new * struct, only the bits that the kernel knows about will be copied. */ - if (copy_to_user(uinfo, &kinfo, min(usize, sizeof(kinfo)))) - return -EFAULT; - - return 0; + return copy_struct_to_user(uinfo, usize, &kinfo, sizeof(kinfo), NULL); } static bool pidfs_ioctl_valid(unsigned int cmd) From b573bf6f693ccb1e62214abf24201e27678b02ad Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Wed, 5 Mar 2025 11:08:12 +0100 Subject: [PATCH 06/25] pidfd: rely on automatic cleanup in __pidfd_prepare() Rely on scope-based cleanup for the allocated file descriptor. Link: https://lore.kernel.org/r/20250305-work-pidfs-kill_on_last_close-v3-2-c8c3d8361705@kernel.org Acked-by: Jeff Layton Reviewed-by: Oleg Nesterov Signed-off-by: Christian Brauner --- kernel/fork.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/kernel/fork.c b/kernel/fork.c index 735405a9c5f3..6230f5256bc5 100644 --- a/kernel/fork.c +++ b/kernel/fork.c @@ -2032,25 +2032,23 @@ static inline void rcu_copy_process(struct task_struct *p) */ static int __pidfd_prepare(struct pid *pid, unsigned int flags, struct file **ret) { - int pidfd; struct file *pidfd_file; - pidfd = get_unused_fd_flags(O_CLOEXEC); + CLASS(get_unused_fd, pidfd)(O_CLOEXEC); if (pidfd < 0) return pidfd; pidfd_file = pidfs_alloc_file(pid, flags | O_RDWR); - if (IS_ERR(pidfd_file)) { - put_unused_fd(pidfd); + if (IS_ERR(pidfd_file)) return PTR_ERR(pidfd_file); - } + /* * anon_inode_getfile() ignores everything outside of the * O_ACCMODE | O_NONBLOCK mask, set PIDFD_THREAD manually. */ pidfd_file->f_flags |= (flags & PIDFD_THREAD); *ret = pidfd_file; - return pidfd; + return take_fd(pidfd); } /** From 3155a194075411528faec9a3b6dede33cab085cc Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Wed, 5 Mar 2025 11:08:13 +0100 Subject: [PATCH 07/25] pidfs: move setting flags into pidfs_alloc_file() Instead od adding it into __pidfd_prepare() place it where the actual file allocation happens and update the outdated comment. Link: https://lore.kernel.org/r/20250305-work-pidfs-kill_on_last_close-v3-3-c8c3d8361705@kernel.org Reviewed-by: Jeff Layton Reviewed-by: Oleg Nesterov Signed-off-by: Christian Brauner --- fs/pidfs.c | 4 ++++ kernel/fork.c | 5 ----- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/fs/pidfs.c b/fs/pidfs.c index aa8c8bda8c8f..ecc0dd886714 100644 --- a/fs/pidfs.c +++ b/fs/pidfs.c @@ -696,6 +696,10 @@ struct file *pidfs_alloc_file(struct pid *pid, unsigned int flags) return ERR_PTR(ret); pidfd_file = dentry_open(&path, flags, current_cred()); + /* Raise PIDFD_THREAD explicitly as do_dentry_open() strips it. */ + if (!IS_ERR(pidfd_file)) + pidfd_file->f_flags |= (flags & PIDFD_THREAD); + path_put(&path); return pidfd_file; } diff --git a/kernel/fork.c b/kernel/fork.c index 6230f5256bc5..8eac9cd3385b 100644 --- a/kernel/fork.c +++ b/kernel/fork.c @@ -2042,11 +2042,6 @@ static int __pidfd_prepare(struct pid *pid, unsigned int flags, struct file **re if (IS_ERR(pidfd_file)) return PTR_ERR(pidfd_file); - /* - * anon_inode_getfile() ignores everything outside of the - * O_ACCMODE | O_NONBLOCK mask, set PIDFD_THREAD manually. - */ - pidfd_file->f_flags |= (flags & PIDFD_THREAD); *ret = pidfd_file; return take_fd(pidfd); } From 0b4200381ed4758391731fa6e84259e1847744c6 Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Wed, 5 Mar 2025 11:08:14 +0100 Subject: [PATCH 08/25] pidfs: use private inode slab cache Introduce a private inode slab cache for pidfs. In follow-up patches pidfs will gain the ability to provide exit information to userspace after the task has been reaped. This means storing exit information even after the task has already been released and struct pid's task linkage is gone. Store that information alongside the inode. Link: https://lore.kernel.org/r/20250305-work-pidfs-kill_on_last_close-v3-4-c8c3d8361705@kernel.org Reviewed-by: Jeff Layton Reviewed-by: Oleg Nesterov Signed-off-by: Christian Brauner --- fs/pidfs.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/fs/pidfs.c b/fs/pidfs.c index ecc0dd886714..282511a36fd9 100644 --- a/fs/pidfs.c +++ b/fs/pidfs.c @@ -24,6 +24,27 @@ #include "internal.h" #include "mount.h" +static struct kmem_cache *pidfs_cachep __ro_after_init; + +/* + * Stashes information that userspace needs to access even after the + * process has been reaped. + */ +struct pidfs_exit_info { + __u64 cgroupid; + __s32 exit_code; +}; + +struct pidfs_inode { + struct pidfs_exit_info exit_info; + struct inode vfs_inode; +}; + +static inline struct pidfs_inode *pidfs_i(struct inode *inode) +{ + return container_of(inode, struct pidfs_inode, vfs_inode); +} + static struct rb_root pidfs_ino_tree = RB_ROOT; #if BITS_PER_LONG == 32 @@ -492,9 +513,29 @@ static void pidfs_evict_inode(struct inode *inode) put_pid(pid); } +static struct inode *pidfs_alloc_inode(struct super_block *sb) +{ + struct pidfs_inode *pi; + + pi = alloc_inode_sb(sb, pidfs_cachep, GFP_KERNEL); + if (!pi) + return NULL; + + memset(&pi->exit_info, 0, sizeof(pi->exit_info)); + + return &pi->vfs_inode; +} + +static void pidfs_free_inode(struct inode *inode) +{ + kmem_cache_free(pidfs_cachep, pidfs_i(inode)); +} + static const struct super_operations pidfs_sops = { + .alloc_inode = pidfs_alloc_inode, .drop_inode = generic_delete_inode, .evict_inode = pidfs_evict_inode, + .free_inode = pidfs_free_inode, .statfs = simple_statfs, }; @@ -704,8 +745,19 @@ struct file *pidfs_alloc_file(struct pid *pid, unsigned int flags) return pidfd_file; } +static void pidfs_inode_init_once(void *data) +{ + struct pidfs_inode *pi = data; + + inode_init_once(&pi->vfs_inode); +} + void __init pidfs_init(void) { + pidfs_cachep = kmem_cache_create("pidfs_cache", sizeof(struct pidfs_inode), 0, + (SLAB_HWCACHE_ALIGN | SLAB_RECLAIM_ACCOUNT | + SLAB_ACCOUNT | SLAB_PANIC), + pidfs_inode_init_once); pidfs_mnt = kern_mount(&pidfs_type); if (IS_ERR(pidfs_mnt)) panic("Failed to mount pidfs pseudo filesystem"); From 4513522984a0af9c170af991c37fbb483cca654b Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Wed, 5 Mar 2025 11:08:15 +0100 Subject: [PATCH 09/25] pidfs: record exit code and cgroupid at exit Record the exit code and cgroupid in release_task() and stash in struct pidfs_exit_info so it can be retrieved even after the task has been reaped. Link: https://lore.kernel.org/r/20250305-work-pidfs-kill_on_last_close-v3-5-c8c3d8361705@kernel.org Reviewed-by: Jeff Layton Reviewed-by: Oleg Nesterov Signed-off-by: Christian Brauner --- fs/internal.h | 1 + fs/libfs.c | 4 ++-- fs/pidfs.c | 41 +++++++++++++++++++++++++++++++++++++++++ include/linux/pidfs.h | 1 + kernel/exit.c | 2 ++ 5 files changed, 47 insertions(+), 2 deletions(-) diff --git a/fs/internal.h b/fs/internal.h index e7f02ae1e098..c1e6d8b294cb 100644 --- a/fs/internal.h +++ b/fs/internal.h @@ -325,6 +325,7 @@ struct stashed_operations { int path_from_stashed(struct dentry **stashed, struct vfsmount *mnt, void *data, struct path *path); void stashed_dentry_prune(struct dentry *dentry); +struct dentry *stashed_dentry_get(struct dentry **stashed); /** * path_mounted - check whether path is mounted * @path: path to check diff --git a/fs/libfs.c b/fs/libfs.c index 8444f5cc4064..cf5a267aafe4 100644 --- a/fs/libfs.c +++ b/fs/libfs.c @@ -2113,7 +2113,7 @@ struct timespec64 simple_inode_init_ts(struct inode *inode) } EXPORT_SYMBOL(simple_inode_init_ts); -static inline struct dentry *get_stashed_dentry(struct dentry **stashed) +struct dentry *stashed_dentry_get(struct dentry **stashed) { struct dentry *dentry; @@ -2215,7 +2215,7 @@ int path_from_stashed(struct dentry **stashed, struct vfsmount *mnt, void *data, const struct stashed_operations *sops = mnt->mnt_sb->s_fs_info; /* See if dentry can be reused. */ - path->dentry = get_stashed_dentry(stashed); + path->dentry = stashed_dentry_get(stashed); if (path->dentry) { sops->put_data(data); goto out_path; diff --git a/fs/pidfs.c b/fs/pidfs.c index 282511a36fd9..c4e6527013e7 100644 --- a/fs/pidfs.c +++ b/fs/pidfs.c @@ -458,6 +458,47 @@ struct pid *pidfd_pid(const struct file *file) return file_inode(file)->i_private; } +/* + * We're called from release_task(). We know there's at least one + * reference to struct pid being held that won't be released until the + * task has been reaped which cannot happen until we're out of + * release_task(). + * + * If this struct pid is referred to by a pidfd then + * stashed_dentry_get() will return the dentry and inode for that struct + * pid. Since we've taken a reference on it there's now an additional + * reference from the exit path on it. Which is fine. We're going to put + * it again in a second and we know that the pid is kept alive anyway. + * + * Worst case is that we've filled in the info and immediately free the + * dentry and inode afterwards since the pidfd has been closed. Since + * pidfs_exit() currently is placed after exit_task_work() we know that + * it cannot be us aka the exiting task holding a pidfd to ourselves. + */ +void pidfs_exit(struct task_struct *tsk) +{ + struct dentry *dentry; + + might_sleep(); + + dentry = stashed_dentry_get(&task_pid(tsk)->stashed); + if (dentry) { + struct inode *inode = d_inode(dentry); + struct pidfs_exit_info *exit_info = &pidfs_i(inode)->exit_info; +#ifdef CONFIG_CGROUPS + struct cgroup *cgrp; + + rcu_read_lock(); + cgrp = task_dfl_cgroup(tsk); + exit_info->cgroupid = cgroup_id(cgrp); + rcu_read_unlock(); +#endif + exit_info->exit_code = tsk->exit_code; + + dput(dentry); + } +} + static struct vfsmount *pidfs_mnt __ro_after_init; /* diff --git a/include/linux/pidfs.h b/include/linux/pidfs.h index 7c830d0dec9a..05e6f8f4a026 100644 --- a/include/linux/pidfs.h +++ b/include/linux/pidfs.h @@ -6,6 +6,7 @@ struct file *pidfs_alloc_file(struct pid *pid, unsigned int flags); void __init pidfs_init(void); void pidfs_add_pid(struct pid *pid); void pidfs_remove_pid(struct pid *pid); +void pidfs_exit(struct task_struct *tsk); extern const struct dentry_operations pidfs_dentry_operations; #endif /* _LINUX_PID_FS_H */ diff --git a/kernel/exit.c b/kernel/exit.c index 3485e5fc499e..9916305e34d3 100644 --- a/kernel/exit.c +++ b/kernel/exit.c @@ -69,6 +69,7 @@ #include #include #include +#include #include @@ -249,6 +250,7 @@ repeat: dec_rlimit_ucounts(task_ucounts(p), UCOUNT_RLIMIT_NPROC, 1); rcu_read_unlock(); + pidfs_exit(p); cgroup_release(p); write_lock_irq(&tasklist_lock); From 7477d7dce48a996ae4e4f0b5f7bd82de7ec9131b Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Wed, 5 Mar 2025 11:08:16 +0100 Subject: [PATCH 10/25] pidfs: allow to retrieve exit information Some tools like systemd's jounral need to retrieve the exit and cgroup information after a process has already been reaped. This can e.g., happen when retrieving a pidfd via SCM_PIDFD or SCM_PEERPIDFD. Link: https://lore.kernel.org/r/20250305-work-pidfs-kill_on_last_close-v3-6-c8c3d8361705@kernel.org Reviewed-by: Jeff Layton Reviewed-by: Oleg Nesterov Signed-off-by: Christian Brauner --- fs/pidfs.c | 86 ++++++++++++++++++++++++++++++-------- include/uapi/linux/pidfd.h | 3 +- 2 files changed, 70 insertions(+), 19 deletions(-) diff --git a/fs/pidfs.c b/fs/pidfs.c index c4e6527013e7..3c630e9d4a62 100644 --- a/fs/pidfs.c +++ b/fs/pidfs.c @@ -36,7 +36,8 @@ struct pidfs_exit_info { }; struct pidfs_inode { - struct pidfs_exit_info exit_info; + struct pidfs_exit_info __pei; + struct pidfs_exit_info *exit_info; struct inode vfs_inode; }; @@ -228,17 +229,28 @@ static __poll_t pidfd_poll(struct file *file, struct poll_table_struct *pts) return poll_flags; } -static long pidfd_info(struct task_struct *task, unsigned int cmd, unsigned long arg) +static inline bool pid_in_current_pidns(const struct pid *pid) +{ + const struct pid_namespace *ns = task_active_pid_ns(current); + + if (ns->level <= pid->level) + return pid->numbers[ns->level].ns == ns; + + return false; +} + +static long pidfd_info(struct file *file, unsigned int cmd, unsigned long arg) { struct pidfd_info __user *uinfo = (struct pidfd_info __user *)arg; + struct inode *inode = file_inode(file); + struct pid *pid = pidfd_pid(file); size_t usize = _IOC_SIZE(cmd); struct pidfd_info kinfo = {}; + struct pidfs_exit_info *exit_info; struct user_namespace *user_ns; + struct task_struct *task; const struct cred *c; __u64 mask; -#ifdef CONFIG_CGROUPS - struct cgroup *cgrp; -#endif if (!uinfo) return -EINVAL; @@ -248,6 +260,37 @@ static long pidfd_info(struct task_struct *task, unsigned int cmd, unsigned long if (copy_from_user(&mask, &uinfo->mask, sizeof(mask))) return -EFAULT; + /* + * Restrict information retrieval to tasks within the caller's pid + * namespace hierarchy. + */ + if (!pid_in_current_pidns(pid)) + return -ESRCH; + + if (mask & PIDFD_INFO_EXIT) { + exit_info = READ_ONCE(pidfs_i(inode)->exit_info); + if (exit_info) { + kinfo.mask |= PIDFD_INFO_EXIT; +#ifdef CONFIG_CGROUPS + kinfo.cgroupid = exit_info->cgroupid; + kinfo.mask |= PIDFD_INFO_CGROUPID; +#endif + kinfo.exit_code = exit_info->exit_code; + } + } + + task = get_pid_task(pid, PIDTYPE_PID); + if (!task) { + /* + * If the task has already been reaped, only exit + * information is available + */ + if (!(mask & PIDFD_INFO_EXIT)) + return -ESRCH; + + goto copy_out; + } + c = get_task_cred(task); if (!c) return -ESRCH; @@ -267,11 +310,15 @@ static long pidfd_info(struct task_struct *task, unsigned int cmd, unsigned long put_cred(c); #ifdef CONFIG_CGROUPS - rcu_read_lock(); - cgrp = task_dfl_cgroup(task); - kinfo.cgroupid = cgroup_id(cgrp); - kinfo.mask |= PIDFD_INFO_CGROUPID; - rcu_read_unlock(); + if (!kinfo.cgroupid) { + struct cgroup *cgrp; + + rcu_read_lock(); + cgrp = task_dfl_cgroup(task); + kinfo.cgroupid = cgroup_id(cgrp); + kinfo.mask |= PIDFD_INFO_CGROUPID; + rcu_read_unlock(); + } #endif /* @@ -291,6 +338,7 @@ static long pidfd_info(struct task_struct *task, unsigned int cmd, unsigned long if (kinfo.pid == 0 || kinfo.tgid == 0 || (kinfo.ppid == 0 && kinfo.pid != 1)) return -ESRCH; +copy_out: /* * If userspace and the kernel have the same struct size it can just * be copied. If userspace provides an older struct, only the bits that @@ -325,7 +373,6 @@ static long pidfd_ioctl(struct file *file, unsigned int cmd, unsigned long arg) { struct task_struct *task __free(put_task) = NULL; struct nsproxy *nsp __free(put_nsproxy) = NULL; - struct pid *pid = pidfd_pid(file); struct ns_common *ns_common = NULL; struct pid_namespace *pid_ns; @@ -340,13 +387,13 @@ static long pidfd_ioctl(struct file *file, unsigned int cmd, unsigned long arg) return put_user(file_inode(file)->i_generation, argp); } - task = get_pid_task(pid, PIDTYPE_PID); - if (!task) - return -ESRCH; - /* Extensible IOCTL that does not open namespace FDs, take a shortcut */ if (_IOC_NR(cmd) == _IOC_NR(PIDFD_GET_INFO)) - return pidfd_info(task, cmd, arg); + return pidfd_info(file, cmd, arg); + + task = get_pid_task(pidfd_pid(file), PIDTYPE_PID); + if (!task) + return -ESRCH; if (arg) return -EINVAL; @@ -484,7 +531,7 @@ void pidfs_exit(struct task_struct *tsk) dentry = stashed_dentry_get(&task_pid(tsk)->stashed); if (dentry) { struct inode *inode = d_inode(dentry); - struct pidfs_exit_info *exit_info = &pidfs_i(inode)->exit_info; + struct pidfs_exit_info *exit_info = &pidfs_i(inode)->__pei; #ifdef CONFIG_CGROUPS struct cgroup *cgrp; @@ -495,6 +542,8 @@ void pidfs_exit(struct task_struct *tsk) #endif exit_info->exit_code = tsk->exit_code; + /* Ensure that PIDFD_GET_INFO sees either all or nothing. */ + smp_store_release(&pidfs_i(inode)->exit_info, &pidfs_i(inode)->__pei); dput(dentry); } } @@ -562,7 +611,8 @@ static struct inode *pidfs_alloc_inode(struct super_block *sb) if (!pi) return NULL; - memset(&pi->exit_info, 0, sizeof(pi->exit_info)); + memset(&pi->__pei, 0, sizeof(pi->__pei)); + pi->exit_info = NULL; return &pi->vfs_inode; } diff --git a/include/uapi/linux/pidfd.h b/include/uapi/linux/pidfd.h index e0abd0b18841..5cd5dcbfe884 100644 --- a/include/uapi/linux/pidfd.h +++ b/include/uapi/linux/pidfd.h @@ -20,6 +20,7 @@ #define PIDFD_INFO_PID (1UL << 0) /* Always returned, even if not requested */ #define PIDFD_INFO_CREDS (1UL << 1) /* Always returned, even if not requested */ #define PIDFD_INFO_CGROUPID (1UL << 2) /* Always returned if available, even if not requested */ +#define PIDFD_INFO_EXIT (1UL << 3) /* Only returned if requested. */ #define PIDFD_INFO_SIZE_VER0 64 /* sizeof first published struct */ @@ -86,7 +87,7 @@ struct pidfd_info { __u32 sgid; __u32 fsuid; __u32 fsgid; - __u32 spare0[1]; + __s32 exit_code; }; #define PIDFS_IOCTL_MAGIC 0xFF From 17457a47d22651970bd74d9c474c1864d8e46f53 Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Wed, 5 Mar 2025 11:08:17 +0100 Subject: [PATCH 11/25] selftests/pidfd: fix header inclusion Ensure that necessary defines are present. Link: https://lore.kernel.org/r/20250305-work-pidfs-kill_on_last_close-v3-7-c8c3d8361705@kernel.org Signed-off-by: Christian Brauner --- tools/testing/selftests/pidfd/pidfd_fdinfo_test.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/testing/selftests/pidfd/pidfd_fdinfo_test.c b/tools/testing/selftests/pidfd/pidfd_fdinfo_test.c index f062a986e382..f718aac75068 100644 --- a/tools/testing/selftests/pidfd/pidfd_fdinfo_test.c +++ b/tools/testing/selftests/pidfd/pidfd_fdinfo_test.c @@ -13,6 +13,7 @@ #include #include #include +#include #include "pidfd.h" #include "../kselftest.h" From 18938f718085c3945cac9482154e6d453d934af1 Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Wed, 5 Mar 2025 11:08:18 +0100 Subject: [PATCH 12/25] pidfs/selftests: ensure correct headers for ioctl handling Ensure that necessary ioctl infrastructure is available. Link: https://lore.kernel.org/r/20250305-work-pidfs-kill_on_last_close-v3-8-c8c3d8361705@kernel.org Signed-off-by: Christian Brauner --- tools/testing/selftests/pidfd/pidfd_setns_test.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/testing/selftests/pidfd/pidfd_setns_test.c b/tools/testing/selftests/pidfd/pidfd_setns_test.c index 222f8131283b..d9e715de68b3 100644 --- a/tools/testing/selftests/pidfd/pidfd_setns_test.c +++ b/tools/testing/selftests/pidfd/pidfd_setns_test.c @@ -16,7 +16,7 @@ #include #include #include -#include +#include #include "pidfd.h" #include "../kselftest_harness.h" From ddf5315526e7ff833b108b9e0069abb33f3ac153 Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Wed, 5 Mar 2025 11:08:19 +0100 Subject: [PATCH 13/25] selftests/pidfd: expand common pidfd header Move more infrastructure to the pidfd header. Link: https://lore.kernel.org/r/20250305-work-pidfs-kill_on_last_close-v3-9-c8c3d8361705@kernel.org Signed-off-by: Christian Brauner --- tools/testing/selftests/pidfd/pidfd.h | 78 +++++++++++++++++++ .../testing/selftests/pidfd/pidfd_open_test.c | 26 ------- .../selftests/pidfd/pidfd_setns_test.c | 45 ----------- 3 files changed, 78 insertions(+), 71 deletions(-) diff --git a/tools/testing/selftests/pidfd/pidfd.h b/tools/testing/selftests/pidfd/pidfd.h index 027ebaf14844..bad518766aa5 100644 --- a/tools/testing/selftests/pidfd/pidfd.h +++ b/tools/testing/selftests/pidfd/pidfd.h @@ -12,6 +12,7 @@ #include #include #include +#include #include #include @@ -66,6 +67,83 @@ #define PIDFD_SELF_PROCESS PIDFD_SELF_THREAD_GROUP #endif +#ifndef PIDFS_IOCTL_MAGIC +#define PIDFS_IOCTL_MAGIC 0xFF +#endif + +#ifndef PIDFD_GET_CGROUP_NAMESPACE +#define PIDFD_GET_CGROUP_NAMESPACE _IO(PIDFS_IOCTL_MAGIC, 1) +#endif + +#ifndef PIDFD_GET_IPC_NAMESPACE +#define PIDFD_GET_IPC_NAMESPACE _IO(PIDFS_IOCTL_MAGIC, 2) +#endif + +#ifndef PIDFD_GET_MNT_NAMESPACE +#define PIDFD_GET_MNT_NAMESPACE _IO(PIDFS_IOCTL_MAGIC, 3) +#endif + +#ifndef PIDFD_GET_NET_NAMESPACE +#define PIDFD_GET_NET_NAMESPACE _IO(PIDFS_IOCTL_MAGIC, 4) +#endif + +#ifndef PIDFD_GET_PID_NAMESPACE +#define PIDFD_GET_PID_NAMESPACE _IO(PIDFS_IOCTL_MAGIC, 5) +#endif + +#ifndef PIDFD_GET_PID_FOR_CHILDREN_NAMESPACE +#define PIDFD_GET_PID_FOR_CHILDREN_NAMESPACE _IO(PIDFS_IOCTL_MAGIC, 6) +#endif + +#ifndef PIDFD_GET_TIME_NAMESPACE +#define PIDFD_GET_TIME_NAMESPACE _IO(PIDFS_IOCTL_MAGIC, 7) +#endif + +#ifndef PIDFD_GET_TIME_FOR_CHILDREN_NAMESPACE +#define PIDFD_GET_TIME_FOR_CHILDREN_NAMESPACE _IO(PIDFS_IOCTL_MAGIC, 8) +#endif + +#ifndef PIDFD_GET_USER_NAMESPACE +#define PIDFD_GET_USER_NAMESPACE _IO(PIDFS_IOCTL_MAGIC, 9) +#endif + +#ifndef PIDFD_GET_UTS_NAMESPACE +#define PIDFD_GET_UTS_NAMESPACE _IO(PIDFS_IOCTL_MAGIC, 10) +#endif + +#ifndef PIDFD_GET_INFO +#define PIDFD_GET_INFO _IOWR(PIDFS_IOCTL_MAGIC, 11, struct pidfd_info) +#endif + +#ifndef PIDFD_INFO_PID +#define PIDFD_INFO_PID (1UL << 0) /* Always returned, even if not requested */ +#endif + +#ifndef PIDFD_INFO_CREDS +#define PIDFD_INFO_CREDS (1UL << 1) /* Always returned, even if not requested */ +#endif + +#ifndef PIDFD_INFO_CGROUPID +#define PIDFD_INFO_CGROUPID (1UL << 2) /* Always returned if available, even if not requested */ +#endif + +struct pidfd_info { + __u64 mask; + __u64 cgroupid; + __u32 pid; + __u32 tgid; + __u32 ppid; + __u32 ruid; + __u32 rgid; + __u32 euid; + __u32 egid; + __u32 suid; + __u32 sgid; + __u32 fsuid; + __u32 fsgid; + __u32 spare0[1]; +}; + /* * The kernel reserves 300 pids via RESERVED_PIDS in kernel/pid.c * That means, when it wraps around any pid < 300 will be skipped. diff --git a/tools/testing/selftests/pidfd/pidfd_open_test.c b/tools/testing/selftests/pidfd/pidfd_open_test.c index 9a40ccb1ff6d..cd3de40e4977 100644 --- a/tools/testing/selftests/pidfd/pidfd_open_test.c +++ b/tools/testing/selftests/pidfd/pidfd_open_test.c @@ -22,32 +22,6 @@ #include "pidfd.h" #include "../kselftest.h" -#ifndef PIDFS_IOCTL_MAGIC -#define PIDFS_IOCTL_MAGIC 0xFF -#endif - -#ifndef PIDFD_GET_INFO -#define PIDFD_GET_INFO _IOWR(PIDFS_IOCTL_MAGIC, 11, struct pidfd_info) -#define PIDFD_INFO_CGROUPID (1UL << 0) - -struct pidfd_info { - __u64 mask; - __u64 cgroupid; - __u32 pid; - __u32 tgid; - __u32 ppid; - __u32 ruid; - __u32 rgid; - __u32 euid; - __u32 egid; - __u32 suid; - __u32 sgid; - __u32 fsuid; - __u32 fsgid; - __u32 spare0[1]; -}; -#endif - static int safe_int(const char *numstr, int *converted) { char *err = NULL; diff --git a/tools/testing/selftests/pidfd/pidfd_setns_test.c b/tools/testing/selftests/pidfd/pidfd_setns_test.c index d9e715de68b3..e6a079b3d5e2 100644 --- a/tools/testing/selftests/pidfd/pidfd_setns_test.c +++ b/tools/testing/selftests/pidfd/pidfd_setns_test.c @@ -16,55 +16,10 @@ #include #include #include -#include #include "pidfd.h" #include "../kselftest_harness.h" -#ifndef PIDFS_IOCTL_MAGIC -#define PIDFS_IOCTL_MAGIC 0xFF -#endif - -#ifndef PIDFD_GET_CGROUP_NAMESPACE -#define PIDFD_GET_CGROUP_NAMESPACE _IO(PIDFS_IOCTL_MAGIC, 1) -#endif - -#ifndef PIDFD_GET_IPC_NAMESPACE -#define PIDFD_GET_IPC_NAMESPACE _IO(PIDFS_IOCTL_MAGIC, 2) -#endif - -#ifndef PIDFD_GET_MNT_NAMESPACE -#define PIDFD_GET_MNT_NAMESPACE _IO(PIDFS_IOCTL_MAGIC, 3) -#endif - -#ifndef PIDFD_GET_NET_NAMESPACE -#define PIDFD_GET_NET_NAMESPACE _IO(PIDFS_IOCTL_MAGIC, 4) -#endif - -#ifndef PIDFD_GET_PID_NAMESPACE -#define PIDFD_GET_PID_NAMESPACE _IO(PIDFS_IOCTL_MAGIC, 5) -#endif - -#ifndef PIDFD_GET_PID_FOR_CHILDREN_NAMESPACE -#define PIDFD_GET_PID_FOR_CHILDREN_NAMESPACE _IO(PIDFS_IOCTL_MAGIC, 6) -#endif - -#ifndef PIDFD_GET_TIME_NAMESPACE -#define PIDFD_GET_TIME_NAMESPACE _IO(PIDFS_IOCTL_MAGIC, 7) -#endif - -#ifndef PIDFD_GET_TIME_FOR_CHILDREN_NAMESPACE -#define PIDFD_GET_TIME_FOR_CHILDREN_NAMESPACE _IO(PIDFS_IOCTL_MAGIC, 8) -#endif - -#ifndef PIDFD_GET_USER_NAMESPACE -#define PIDFD_GET_USER_NAMESPACE _IO(PIDFS_IOCTL_MAGIC, 9) -#endif - -#ifndef PIDFD_GET_UTS_NAMESPACE -#define PIDFD_GET_UTS_NAMESPACE _IO(PIDFS_IOCTL_MAGIC, 10) -#endif - enum { PIDFD_NS_USER, PIDFD_NS_MNT, From 853ab1ff2cf9b61e26a3029a895a725837f83e18 Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Wed, 5 Mar 2025 11:08:20 +0100 Subject: [PATCH 14/25] selftests/pidfd: add first PIDFD_INFO_EXIT selftest Add a selftest for PIDFD_INFO_EXIT behavior. Link: https://lore.kernel.org/r/20250305-work-pidfs-kill_on_last_close-v3-10-c8c3d8361705@kernel.org Signed-off-by: Christian Brauner --- tools/testing/selftests/pidfd/.gitignore | 1 + tools/testing/selftests/pidfd/Makefile | 2 +- tools/testing/selftests/pidfd/pidfd.h | 6 +- .../testing/selftests/pidfd/pidfd_info_test.c | 146 ++++++++++++++++++ 4 files changed, 153 insertions(+), 2 deletions(-) create mode 100644 tools/testing/selftests/pidfd/pidfd_info_test.c diff --git a/tools/testing/selftests/pidfd/.gitignore b/tools/testing/selftests/pidfd/.gitignore index bf92481f925c..bddae1d4d7e4 100644 --- a/tools/testing/selftests/pidfd/.gitignore +++ b/tools/testing/selftests/pidfd/.gitignore @@ -8,3 +8,4 @@ pidfd_getfd_test pidfd_setns_test pidfd_file_handle_test pidfd_bind_mount +pidfd_info_test diff --git a/tools/testing/selftests/pidfd/Makefile b/tools/testing/selftests/pidfd/Makefile index 301343a11b62..a94c2bc8d594 100644 --- a/tools/testing/selftests/pidfd/Makefile +++ b/tools/testing/selftests/pidfd/Makefile @@ -3,7 +3,7 @@ CFLAGS += -g $(KHDR_INCLUDES) -pthread -Wall TEST_GEN_PROGS := pidfd_test pidfd_fdinfo_test pidfd_open_test \ pidfd_poll_test pidfd_wait pidfd_getfd_test pidfd_setns_test \ - pidfd_file_handle_test pidfd_bind_mount + pidfd_file_handle_test pidfd_bind_mount pidfd_info_test include ../lib.mk diff --git a/tools/testing/selftests/pidfd/pidfd.h b/tools/testing/selftests/pidfd/pidfd.h index bad518766aa5..cc8e381978df 100644 --- a/tools/testing/selftests/pidfd/pidfd.h +++ b/tools/testing/selftests/pidfd/pidfd.h @@ -127,6 +127,10 @@ #define PIDFD_INFO_CGROUPID (1UL << 2) /* Always returned if available, even if not requested */ #endif +#ifndef PIDFD_INFO_EXIT +#define PIDFD_INFO_EXIT (1UL << 3) /* Always returned if available, even if not requested */ +#endif + struct pidfd_info { __u64 mask; __u64 cgroupid; @@ -141,7 +145,7 @@ struct pidfd_info { __u32 sgid; __u32 fsuid; __u32 fsgid; - __u32 spare0[1]; + __s32 exit_code; }; /* diff --git a/tools/testing/selftests/pidfd/pidfd_info_test.c b/tools/testing/selftests/pidfd/pidfd_info_test.c new file mode 100644 index 000000000000..cc1d3d5eba59 --- /dev/null +++ b/tools/testing/selftests/pidfd/pidfd_info_test.c @@ -0,0 +1,146 @@ +// SPDX-License-Identifier: GPL-2.0 + +#define _GNU_SOURCE +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "pidfd.h" +#include "../kselftest_harness.h" + +FIXTURE(pidfd_info) +{ + pid_t child_pid1; + int child_pidfd1; + + pid_t child_pid2; + int child_pidfd2; + + pid_t child_pid3; + int child_pidfd3; + + pid_t child_pid4; + int child_pidfd4; +}; + +FIXTURE_SETUP(pidfd_info) +{ + int ret; + int ipc_sockets[2]; + char c; + + ret = socketpair(AF_LOCAL, SOCK_STREAM | SOCK_CLOEXEC, 0, ipc_sockets); + EXPECT_EQ(ret, 0); + + self->child_pid1 = create_child(&self->child_pidfd1, 0); + EXPECT_GE(self->child_pid1, 0); + + if (self->child_pid1 == 0) { + close(ipc_sockets[0]); + + if (write_nointr(ipc_sockets[1], "1", 1) < 0) + _exit(EXIT_FAILURE); + + close(ipc_sockets[1]); + + pause(); + _exit(EXIT_SUCCESS); + } + + EXPECT_EQ(close(ipc_sockets[1]), 0); + ASSERT_EQ(read_nointr(ipc_sockets[0], &c, 1), 1); + EXPECT_EQ(close(ipc_sockets[0]), 0); + + /* SIGKILL but don't reap. */ + EXPECT_EQ(sys_pidfd_send_signal(self->child_pidfd1, SIGKILL, NULL, 0), 0); + + ret = socketpair(AF_LOCAL, SOCK_STREAM | SOCK_CLOEXEC, 0, ipc_sockets); + EXPECT_EQ(ret, 0); + + self->child_pid2 = create_child(&self->child_pidfd2, 0); + EXPECT_GE(self->child_pid2, 0); + + if (self->child_pid2 == 0) { + close(ipc_sockets[0]); + + if (write_nointr(ipc_sockets[1], "1", 1) < 0) + _exit(EXIT_FAILURE); + + close(ipc_sockets[1]); + + pause(); + _exit(EXIT_SUCCESS); + } + + EXPECT_EQ(close(ipc_sockets[1]), 0); + ASSERT_EQ(read_nointr(ipc_sockets[0], &c, 1), 1); + EXPECT_EQ(close(ipc_sockets[0]), 0); + + /* SIGKILL and reap. */ + EXPECT_EQ(sys_pidfd_send_signal(self->child_pidfd2, SIGKILL, NULL, 0), 0); + EXPECT_EQ(sys_waitid(P_PID, self->child_pid2, NULL, WEXITED), 0); + + self->child_pid3 = create_child(&self->child_pidfd3, CLONE_NEWUSER | CLONE_NEWPID); + EXPECT_GE(self->child_pid3, 0); + + if (self->child_pid3 == 0) + _exit(EXIT_SUCCESS); + + self->child_pid4 = create_child(&self->child_pidfd4, CLONE_NEWUSER | CLONE_NEWPID); + EXPECT_GE(self->child_pid4, 0); + + if (self->child_pid4 == 0) + _exit(EXIT_SUCCESS); + + EXPECT_EQ(sys_waitid(P_PID, self->child_pid4, NULL, WEXITED), 0); +} + +FIXTURE_TEARDOWN(pidfd_info) +{ + sys_pidfd_send_signal(self->child_pidfd1, SIGKILL, NULL, 0); + if (self->child_pidfd1 >= 0) + EXPECT_EQ(0, close(self->child_pidfd1)); + + sys_waitid(P_PID, self->child_pid1, NULL, WEXITED); + + sys_pidfd_send_signal(self->child_pidfd2, SIGKILL, NULL, 0); + if (self->child_pidfd2 >= 0) + EXPECT_EQ(0, close(self->child_pidfd2)); + + sys_waitid(P_PID, self->child_pid2, NULL, WEXITED); + sys_waitid(P_PID, self->child_pid3, NULL, WEXITED); + sys_waitid(P_PID, self->child_pid4, NULL, WEXITED); +} + +TEST_F(pidfd_info, sigkill_exit) +{ + struct pidfd_info info = { + .mask = PIDFD_INFO_CGROUPID, + }; + + /* Process has exited but not been reaped so this must work. */ + ASSERT_EQ(ioctl(self->child_pidfd1, PIDFD_GET_INFO, &info), 0); + + info.mask = PIDFD_INFO_CGROUPID | PIDFD_INFO_EXIT; + ASSERT_EQ(ioctl(self->child_pidfd1, PIDFD_GET_INFO, &info), 0); + ASSERT_TRUE(!!(info.mask & PIDFD_INFO_CREDS)); + /* Process has exited but not been reaped, so no PIDFD_INFO_EXIT information yet. */ + ASSERT_FALSE(!!(info.mask & PIDFD_INFO_EXIT)); +} + +TEST_HARNESS_MAIN From 86c1dfdd523ca52dd0c72d2039e77b66fffb6ef7 Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Wed, 5 Mar 2025 11:08:21 +0100 Subject: [PATCH 15/25] selftests/pidfd: add second PIDFD_INFO_EXIT selftest Add a selftest for PIDFD_INFO_EXIT behavior. Link: https://lore.kernel.org/r/20250305-work-pidfs-kill_on_last_close-v3-11-c8c3d8361705@kernel.org Signed-off-by: Christian Brauner --- .../testing/selftests/pidfd/pidfd_info_test.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/tools/testing/selftests/pidfd/pidfd_info_test.c b/tools/testing/selftests/pidfd/pidfd_info_test.c index cc1d3d5eba59..2a5742a2a55f 100644 --- a/tools/testing/selftests/pidfd/pidfd_info_test.c +++ b/tools/testing/selftests/pidfd/pidfd_info_test.c @@ -143,4 +143,22 @@ TEST_F(pidfd_info, sigkill_exit) ASSERT_FALSE(!!(info.mask & PIDFD_INFO_EXIT)); } +TEST_F(pidfd_info, sigkill_reaped) +{ + struct pidfd_info info = { + .mask = PIDFD_INFO_CGROUPID, + }; + + /* Process has already been reaped and PIDFD_INFO_EXIT hasn't been set. */ + ASSERT_NE(ioctl(self->child_pidfd2, PIDFD_GET_INFO, &info), 0); + ASSERT_EQ(errno, ESRCH); + + info.mask = PIDFD_INFO_CGROUPID | PIDFD_INFO_EXIT; + ASSERT_EQ(ioctl(self->child_pidfd2, PIDFD_GET_INFO, &info), 0); + ASSERT_FALSE(!!(info.mask & PIDFD_INFO_CREDS)); + ASSERT_TRUE(!!(info.mask & PIDFD_INFO_EXIT)); + ASSERT_TRUE(WIFSIGNALED(info.exit_code)); + ASSERT_EQ(WTERMSIG(info.exit_code), SIGKILL); +} + TEST_HARNESS_MAIN From a79975f05e5b3c38a4aa215809e0d04928451ad9 Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Wed, 5 Mar 2025 11:08:22 +0100 Subject: [PATCH 16/25] selftests/pidfd: add third PIDFD_INFO_EXIT selftest Add a selftest for PIDFD_INFO_EXIT behavior. Link: https://lore.kernel.org/r/20250305-work-pidfs-kill_on_last_close-v3-12-c8c3d8361705@kernel.org Signed-off-by: Christian Brauner --- tools/testing/selftests/pidfd/pidfd_info_test.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tools/testing/selftests/pidfd/pidfd_info_test.c b/tools/testing/selftests/pidfd/pidfd_info_test.c index 2a5742a2a55f..2917e7a03b31 100644 --- a/tools/testing/selftests/pidfd/pidfd_info_test.c +++ b/tools/testing/selftests/pidfd/pidfd_info_test.c @@ -161,4 +161,20 @@ TEST_F(pidfd_info, sigkill_reaped) ASSERT_EQ(WTERMSIG(info.exit_code), SIGKILL); } +TEST_F(pidfd_info, success_exit) +{ + struct pidfd_info info = { + .mask = PIDFD_INFO_CGROUPID, + }; + + /* Process has exited but not been reaped so this must work. */ + ASSERT_EQ(ioctl(self->child_pidfd3, PIDFD_GET_INFO, &info), 0); + + info.mask = PIDFD_INFO_CGROUPID | PIDFD_INFO_EXIT; + ASSERT_EQ(ioctl(self->child_pidfd3, PIDFD_GET_INFO, &info), 0); + ASSERT_TRUE(!!(info.mask & PIDFD_INFO_CREDS)); + /* Process has exited but not been reaped, so no PIDFD_INFO_EXIT information yet. */ + ASSERT_FALSE(!!(info.mask & PIDFD_INFO_EXIT)); +} + TEST_HARNESS_MAIN From 2e94e4c649c8802fcb9e348d9774e763169ba246 Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Wed, 5 Mar 2025 11:08:23 +0100 Subject: [PATCH 17/25] selftests/pidfd: add fourth PIDFD_INFO_EXIT selftest Add a selftest for PIDFD_INFO_EXIT behavior. Link: https://lore.kernel.org/r/20250305-work-pidfs-kill_on_last_close-v3-13-c8c3d8361705@kernel.org Signed-off-by: Christian Brauner --- .../testing/selftests/pidfd/pidfd_info_test.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/tools/testing/selftests/pidfd/pidfd_info_test.c b/tools/testing/selftests/pidfd/pidfd_info_test.c index 2917e7a03b31..0d0af4c2a84d 100644 --- a/tools/testing/selftests/pidfd/pidfd_info_test.c +++ b/tools/testing/selftests/pidfd/pidfd_info_test.c @@ -177,4 +177,22 @@ TEST_F(pidfd_info, success_exit) ASSERT_FALSE(!!(info.mask & PIDFD_INFO_EXIT)); } +TEST_F(pidfd_info, success_reaped) +{ + struct pidfd_info info = { + .mask = PIDFD_INFO_CGROUPID, + }; + + /* Process has already been reaped and PIDFD_INFO_EXIT hasn't been set. */ + ASSERT_NE(ioctl(self->child_pidfd4, PIDFD_GET_INFO, &info), 0); + ASSERT_EQ(errno, ESRCH); + + info.mask = PIDFD_INFO_CGROUPID | PIDFD_INFO_EXIT; + ASSERT_EQ(ioctl(self->child_pidfd4, PIDFD_GET_INFO, &info), 0); + ASSERT_FALSE(!!(info.mask & PIDFD_INFO_CREDS)); + ASSERT_TRUE(!!(info.mask & PIDFD_INFO_EXIT)); + ASSERT_TRUE(WIFEXITED(info.exit_code)); + ASSERT_EQ(WEXITSTATUS(info.exit_code), 0); +} + TEST_HARNESS_MAIN From 2adf6ca63871962a353d627c692a23d7d89045c7 Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Wed, 5 Mar 2025 11:08:24 +0100 Subject: [PATCH 18/25] selftests/pidfd: add fifth PIDFD_INFO_EXIT selftest Add a selftest for PIDFD_INFO_EXIT behavior. Link: https://lore.kernel.org/r/20250305-work-pidfs-kill_on_last_close-v3-14-c8c3d8361705@kernel.org Signed-off-by: Christian Brauner --- .../testing/selftests/pidfd/pidfd_info_test.c | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/tools/testing/selftests/pidfd/pidfd_info_test.c b/tools/testing/selftests/pidfd/pidfd_info_test.c index 0d0af4c2a84d..16e4be2364df 100644 --- a/tools/testing/selftests/pidfd/pidfd_info_test.c +++ b/tools/testing/selftests/pidfd/pidfd_info_test.c @@ -195,4 +195,27 @@ TEST_F(pidfd_info, success_reaped) ASSERT_EQ(WEXITSTATUS(info.exit_code), 0); } +TEST_F(pidfd_info, success_reaped_poll) +{ + struct pidfd_info info = { + .mask = PIDFD_INFO_CGROUPID | PIDFD_INFO_EXIT, + }; + struct pollfd fds = {}; + int nevents; + + fds.events = POLLIN; + fds.fd = self->child_pidfd2; + + nevents = poll(&fds, 1, -1); + ASSERT_EQ(nevents, 1); + ASSERT_TRUE(!!(fds.revents & POLLIN)); + ASSERT_TRUE(!!(fds.revents & POLLHUP)); + + ASSERT_EQ(ioctl(self->child_pidfd2, PIDFD_GET_INFO, &info), 0); + ASSERT_FALSE(!!(info.mask & PIDFD_INFO_CREDS)); + ASSERT_TRUE(!!(info.mask & PIDFD_INFO_EXIT)); + ASSERT_TRUE(WIFSIGNALED(info.exit_code)); + ASSERT_EQ(WTERMSIG(info.exit_code), SIGKILL); +} + TEST_HARNESS_MAIN From 1c4f2dbe42821734d227c392f968a3a8491f3efe Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Wed, 5 Mar 2025 11:08:25 +0100 Subject: [PATCH 19/25] selftests/pidfd: add sixth PIDFD_INFO_EXIT selftest Add a selftest for PIDFD_INFO_EXIT behavior. Link: https://lore.kernel.org/r/20250305-work-pidfs-kill_on_last_close-v3-15-c8c3d8361705@kernel.org Signed-off-by: Christian Brauner --- tools/testing/selftests/pidfd/pidfd.h | 4 + .../testing/selftests/pidfd/pidfd_info_test.c | 151 ++++++++++++++++++ 2 files changed, 155 insertions(+) diff --git a/tools/testing/selftests/pidfd/pidfd.h b/tools/testing/selftests/pidfd/pidfd.h index cc8e381978df..fee6fd3e67dd 100644 --- a/tools/testing/selftests/pidfd/pidfd.h +++ b/tools/testing/selftests/pidfd/pidfd.h @@ -131,6 +131,10 @@ #define PIDFD_INFO_EXIT (1UL << 3) /* Always returned if available, even if not requested */ #endif +#ifndef PIDFD_THREAD +#define PIDFD_THREAD O_EXCL +#endif + struct pidfd_info { __u64 mask; __u64 cgroupid; diff --git a/tools/testing/selftests/pidfd/pidfd_info_test.c b/tools/testing/selftests/pidfd/pidfd_info_test.c index 16e4be2364df..5e86e3df323b 100644 --- a/tools/testing/selftests/pidfd/pidfd_info_test.c +++ b/tools/testing/selftests/pidfd/pidfd_info_test.c @@ -218,4 +218,155 @@ TEST_F(pidfd_info, success_reaped_poll) ASSERT_EQ(WTERMSIG(info.exit_code), SIGKILL); } +static void *pidfd_info_pause_thread(void *arg) +{ + pid_t pid_thread = gettid(); + int ipc_socket = *(int *)arg; + + /* Inform the grand-parent what the tid of this thread is. */ + if (write_nointr(ipc_socket, &pid_thread, sizeof(pid_thread)) != sizeof(pid_thread)) + return NULL; + + close(ipc_socket); + + /* Sleep untill we're killed. */ + pause(); + return NULL; +} + +TEST_F(pidfd_info, thread_group) +{ + pid_t pid_leader, pid_thread; + pthread_t thread; + int nevents, pidfd_leader, pidfd_thread, pidfd_leader_thread, ret; + int ipc_sockets[2]; + struct pollfd fds = {}; + struct pidfd_info info = { + .mask = PIDFD_INFO_CGROUPID | PIDFD_INFO_EXIT, + }, info2; + + ret = socketpair(AF_LOCAL, SOCK_STREAM | SOCK_CLOEXEC, 0, ipc_sockets); + EXPECT_EQ(ret, 0); + + pid_leader = create_child(&pidfd_leader, 0); + EXPECT_GE(pid_leader, 0); + + if (pid_leader == 0) { + close(ipc_sockets[0]); + + /* The thread will outlive the thread-group leader. */ + if (pthread_create(&thread, NULL, pidfd_info_pause_thread, &ipc_sockets[1])) + syscall(__NR_exit, EXIT_FAILURE); + + /* Make the thread-group leader exit prematurely. */ + syscall(__NR_exit, EXIT_SUCCESS); + } + + /* Retrieve the tid of the thread. */ + EXPECT_EQ(close(ipc_sockets[1]), 0); + ASSERT_EQ(read_nointr(ipc_sockets[0], &pid_thread, sizeof(pid_thread)), sizeof(pid_thread)); + EXPECT_EQ(close(ipc_sockets[0]), 0); + + /* Opening a thread as a thread-group leader must fail. */ + pidfd_thread = sys_pidfd_open(pid_thread, 0); + ASSERT_LT(pidfd_thread, 0); + + /* Opening a thread as a PIDFD_THREAD must succeed. */ + pidfd_thread = sys_pidfd_open(pid_thread, PIDFD_THREAD); + ASSERT_GE(pidfd_thread, 0); + + /* + * Opening a PIDFD_THREAD aka thread-specific pidfd based on a + * thread-group leader must succeed. + */ + pidfd_leader_thread = sys_pidfd_open(pid_leader, PIDFD_THREAD); + ASSERT_GE(pidfd_leader_thread, 0); + + /* + * Note that pidfd_leader is a thread-group pidfd, so polling on it + * would only notify us once all thread in the thread-group have + * exited. So we can't poll before we have taken down the whole + * thread-group. + */ + + /* Get PIDFD_GET_INFO using the thread-group leader pidfd. */ + ASSERT_EQ(ioctl(pidfd_leader, PIDFD_GET_INFO, &info), 0); + ASSERT_TRUE(!!(info.mask & PIDFD_INFO_CREDS)); + /* Process has exited but not been reaped, so no PIDFD_INFO_EXIT information yet. */ + ASSERT_FALSE(!!(info.mask & PIDFD_INFO_EXIT)); + ASSERT_EQ(info.pid, pid_leader); + + /* + * Now retrieve the same info using the thread specific pidfd + * for the thread-group leader. + */ + info2.mask = PIDFD_INFO_CGROUPID | PIDFD_INFO_EXIT; + ASSERT_EQ(ioctl(pidfd_leader_thread, PIDFD_GET_INFO, &info2), 0); + ASSERT_TRUE(!!(info2.mask & PIDFD_INFO_CREDS)); + /* Process has exited but not been reaped, so no PIDFD_INFO_EXIT information yet. */ + ASSERT_FALSE(!!(info2.mask & PIDFD_INFO_EXIT)); + ASSERT_EQ(info2.pid, pid_leader); + + /* Now try the thread-specific pidfd. */ + ASSERT_EQ(ioctl(pidfd_thread, PIDFD_GET_INFO, &info), 0); + ASSERT_TRUE(!!(info.mask & PIDFD_INFO_CREDS)); + /* The thread hasn't exited, so no PIDFD_INFO_EXIT information yet. */ + ASSERT_FALSE(!!(info.mask & PIDFD_INFO_EXIT)); + ASSERT_EQ(info.pid, pid_thread); + + /* + * Take down the whole thread-group. The thread-group leader + * exited successfully but the thread will now be SIGKILLed. + * This must be reflected in the recorded exit information. + */ + EXPECT_EQ(sys_pidfd_send_signal(pidfd_leader, SIGKILL, NULL, 0), 0); + EXPECT_EQ(sys_waitid(P_PIDFD, pidfd_leader, NULL, WEXITED), 0); + + fds.events = POLLIN; + fds.fd = pidfd_leader; + nevents = poll(&fds, 1, -1); + ASSERT_EQ(nevents, 1); + ASSERT_TRUE(!!(fds.revents & POLLIN)); + /* The thread-group leader has been reaped. */ + ASSERT_TRUE(!!(fds.revents & POLLHUP)); + + /* + * Retrieve exit information for the thread-group leader via the + * thread-group leader pidfd. + */ + info.mask = PIDFD_INFO_CGROUPID | PIDFD_INFO_EXIT; + ASSERT_EQ(ioctl(pidfd_leader, PIDFD_GET_INFO, &info), 0); + ASSERT_FALSE(!!(info.mask & PIDFD_INFO_CREDS)); + ASSERT_TRUE(!!(info.mask & PIDFD_INFO_EXIT)); + /* The thread-group leader exited successfully. Only the specific thread was SIGKILLed. */ + ASSERT_TRUE(WIFEXITED(info.exit_code)); + ASSERT_EQ(WEXITSTATUS(info.exit_code), 0); + + /* + * Retrieve exit information for the thread-group leader via the + * thread-specific pidfd. + */ + info2.mask = PIDFD_INFO_CGROUPID | PIDFD_INFO_EXIT; + ASSERT_EQ(ioctl(pidfd_leader_thread, PIDFD_GET_INFO, &info2), 0); + ASSERT_FALSE(!!(info2.mask & PIDFD_INFO_CREDS)); + ASSERT_TRUE(!!(info2.mask & PIDFD_INFO_EXIT)); + + /* The thread-group leader exited successfully. Only the specific thread was SIGKILLed. */ + ASSERT_TRUE(WIFEXITED(info2.exit_code)); + ASSERT_EQ(WEXITSTATUS(info2.exit_code), 0); + + /* Retrieve exit information for the thread. */ + info.mask = PIDFD_INFO_CGROUPID | PIDFD_INFO_EXIT; + ASSERT_EQ(ioctl(pidfd_thread, PIDFD_GET_INFO, &info), 0); + ASSERT_FALSE(!!(info.mask & PIDFD_INFO_CREDS)); + ASSERT_TRUE(!!(info.mask & PIDFD_INFO_EXIT)); + + /* The thread got SIGKILLed. */ + ASSERT_TRUE(WIFSIGNALED(info.exit_code)); + ASSERT_EQ(WTERMSIG(info.exit_code), SIGKILL); + + EXPECT_EQ(close(pidfd_leader), 0); + EXPECT_EQ(close(pidfd_thread), 0); +} + TEST_HARNESS_MAIN From 56f235da15d02c550c0ae25da9c62ecfe7222d38 Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Wed, 5 Mar 2025 11:08:26 +0100 Subject: [PATCH 20/25] selftests/pidfd: add seventh PIDFD_INFO_EXIT selftest Add a selftest for PIDFD_INFO_EXIT behavior. Link: https://lore.kernel.org/r/20250305-work-pidfs-kill_on_last_close-v3-16-c8c3d8361705@kernel.org Signed-off-by: Christian Brauner --- tools/testing/selftests/pidfd/.gitignore | 1 + tools/testing/selftests/pidfd/Makefile | 2 + tools/testing/selftests/pidfd/pidfd.h | 7 + .../selftests/pidfd/pidfd_exec_helper.c | 12 ++ .../testing/selftests/pidfd/pidfd_info_test.c | 125 ++++++++++++++++++ 5 files changed, 147 insertions(+) create mode 100644 tools/testing/selftests/pidfd/pidfd_exec_helper.c diff --git a/tools/testing/selftests/pidfd/.gitignore b/tools/testing/selftests/pidfd/.gitignore index bddae1d4d7e4..0406a065deb4 100644 --- a/tools/testing/selftests/pidfd/.gitignore +++ b/tools/testing/selftests/pidfd/.gitignore @@ -9,3 +9,4 @@ pidfd_setns_test pidfd_file_handle_test pidfd_bind_mount pidfd_info_test +pidfd_exec_helper diff --git a/tools/testing/selftests/pidfd/Makefile b/tools/testing/selftests/pidfd/Makefile index a94c2bc8d594..fcbefc0d77f6 100644 --- a/tools/testing/selftests/pidfd/Makefile +++ b/tools/testing/selftests/pidfd/Makefile @@ -5,5 +5,7 @@ TEST_GEN_PROGS := pidfd_test pidfd_fdinfo_test pidfd_open_test \ pidfd_poll_test pidfd_wait pidfd_getfd_test pidfd_setns_test \ pidfd_file_handle_test pidfd_bind_mount pidfd_info_test +TEST_GEN_PROGS_EXTENDED := pidfd_exec_helper + include ../lib.mk diff --git a/tools/testing/selftests/pidfd/pidfd.h b/tools/testing/selftests/pidfd/pidfd.h index fee6fd3e67dd..cec22aa11cdf 100644 --- a/tools/testing/selftests/pidfd/pidfd.h +++ b/tools/testing/selftests/pidfd/pidfd.h @@ -254,4 +254,11 @@ static inline ssize_t write_nointr(int fd, const void *buf, size_t count) return ret; } +static inline int sys_execveat(int dirfd, const char *pathname, + char *const argv[], char *const envp[], + int flags) +{ + return syscall(__NR_execveat, dirfd, pathname, argv, envp, flags); +} + #endif /* __PIDFD_H */ diff --git a/tools/testing/selftests/pidfd/pidfd_exec_helper.c b/tools/testing/selftests/pidfd/pidfd_exec_helper.c new file mode 100644 index 000000000000..5516808c95f2 --- /dev/null +++ b/tools/testing/selftests/pidfd/pidfd_exec_helper.c @@ -0,0 +1,12 @@ +#define _GNU_SOURCE +#include +#include +#include + +int main(int argc, char *argv[]) +{ + if (pause()) + _exit(EXIT_FAILURE); + + _exit(EXIT_SUCCESS); +} diff --git a/tools/testing/selftests/pidfd/pidfd_info_test.c b/tools/testing/selftests/pidfd/pidfd_info_test.c index 5e86e3df323b..09bc4ae7aed5 100644 --- a/tools/testing/selftests/pidfd/pidfd_info_test.c +++ b/tools/testing/selftests/pidfd/pidfd_info_test.c @@ -369,4 +369,129 @@ TEST_F(pidfd_info, thread_group) EXPECT_EQ(close(pidfd_thread), 0); } +static void *pidfd_info_thread_exec(void *arg) +{ + pid_t pid_thread = gettid(); + int ipc_socket = *(int *)arg; + + /* Inform the grand-parent what the tid of this thread is. */ + if (write_nointr(ipc_socket, &pid_thread, sizeof(pid_thread)) != sizeof(pid_thread)) + return NULL; + + if (read_nointr(ipc_socket, &pid_thread, sizeof(pid_thread)) != sizeof(pid_thread)) + return NULL; + + close(ipc_socket); + + sys_execveat(AT_FDCWD, "pidfd_exec_helper", NULL, NULL, 0); + return NULL; +} + +TEST_F(pidfd_info, thread_group_exec) +{ + pid_t pid_leader, pid_thread; + pthread_t thread; + int nevents, pidfd_leader, pidfd_leader_thread, pidfd_thread, ret; + int ipc_sockets[2]; + struct pollfd fds = {}; + struct pidfd_info info = { + .mask = PIDFD_INFO_CGROUPID | PIDFD_INFO_EXIT, + }; + + ret = socketpair(AF_LOCAL, SOCK_STREAM | SOCK_CLOEXEC, 0, ipc_sockets); + EXPECT_EQ(ret, 0); + + pid_leader = create_child(&pidfd_leader, 0); + EXPECT_GE(pid_leader, 0); + + if (pid_leader == 0) { + close(ipc_sockets[0]); + + /* The thread will outlive the thread-group leader. */ + if (pthread_create(&thread, NULL, pidfd_info_thread_exec, &ipc_sockets[1])) + syscall(__NR_exit, EXIT_FAILURE); + + /* Make the thread-group leader exit prematurely. */ + syscall(__NR_exit, EXIT_SUCCESS); + } + + /* Retrieve the tid of the thread. */ + EXPECT_EQ(close(ipc_sockets[1]), 0); + ASSERT_EQ(read_nointr(ipc_sockets[0], &pid_thread, sizeof(pid_thread)), sizeof(pid_thread)); + + /* Opening a thread as a PIDFD_THREAD must succeed. */ + pidfd_thread = sys_pidfd_open(pid_thread, PIDFD_THREAD); + ASSERT_GE(pidfd_thread, 0); + + /* Open a thread-specific pidfd for the thread-group leader. */ + pidfd_leader_thread = sys_pidfd_open(pid_leader, PIDFD_THREAD); + ASSERT_GE(pidfd_leader_thread, 0); + + /* + * We can poll and wait for the old thread-group leader to exit + * using a thread-specific pidfd. + * + * This only works until the thread has execed. When the thread + * has execed it will have taken over the old thread-group + * leaders struct pid. Calling poll after the thread execed will + * thus block again because a new thread-group has started (Yes, + * it's fscked.). + */ + fds.events = POLLIN; + fds.fd = pidfd_leader_thread; + nevents = poll(&fds, 1, -1); + ASSERT_EQ(nevents, 1); + /* The thread-group leader has exited. */ + ASSERT_TRUE(!!(fds.revents & POLLIN)); + /* The thread-group leader hasn't been reaped. */ + ASSERT_FALSE(!!(fds.revents & POLLHUP)); + + /* Now that we've opened a thread-specific pidfd the thread can exec. */ + ASSERT_EQ(write_nointr(ipc_sockets[0], &pid_thread, sizeof(pid_thread)), sizeof(pid_thread)); + EXPECT_EQ(close(ipc_sockets[0]), 0); + + /* Wait until the kernel has SIGKILLed the thread. */ + fds.events = POLLHUP; + fds.fd = pidfd_thread; + nevents = poll(&fds, 1, -1); + ASSERT_EQ(nevents, 1); + /* The thread has been reaped. */ + ASSERT_TRUE(!!(fds.revents & POLLHUP)); + + /* Retrieve thread-specific exit info from pidfd. */ + ASSERT_EQ(ioctl(pidfd_thread, PIDFD_GET_INFO, &info), 0); + ASSERT_FALSE(!!(info.mask & PIDFD_INFO_CREDS)); + ASSERT_TRUE(!!(info.mask & PIDFD_INFO_EXIT)); + /* + * While the kernel will have SIGKILLed the whole thread-group + * during exec it will cause the individual threads to exit + * cleanly. + */ + ASSERT_TRUE(WIFEXITED(info.exit_code)); + ASSERT_EQ(WEXITSTATUS(info.exit_code), 0); + + /* + * The thread-group leader is still alive, the thread has taken + * over its struct pid and thus its pid number. + */ + info.mask = PIDFD_INFO_CGROUPID | PIDFD_INFO_EXIT; + ASSERT_EQ(ioctl(pidfd_leader, PIDFD_GET_INFO, &info), 0); + ASSERT_TRUE(!!(info.mask & PIDFD_INFO_CREDS)); + ASSERT_FALSE(!!(info.mask & PIDFD_INFO_EXIT)); + ASSERT_EQ(info.pid, pid_leader); + + /* Take down the thread-group leader. */ + EXPECT_EQ(sys_pidfd_send_signal(pidfd_leader, SIGKILL, NULL, 0), 0); + EXPECT_EQ(sys_waitid(P_PIDFD, pidfd_leader, NULL, WEXITED), 0); + + /* Retrieve exit information for the thread-group leader. */ + info.mask = PIDFD_INFO_CGROUPID | PIDFD_INFO_EXIT; + ASSERT_EQ(ioctl(pidfd_leader, PIDFD_GET_INFO, &info), 0); + ASSERT_FALSE(!!(info.mask & PIDFD_INFO_CREDS)); + ASSERT_TRUE(!!(info.mask & PIDFD_INFO_EXIT)); + + EXPECT_EQ(close(pidfd_leader), 0); + EXPECT_EQ(close(pidfd_thread), 0); +} + TEST_HARNESS_MAIN From 68db272741834d5e528b5e1af6b83b479fec6cbb Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Sun, 16 Mar 2025 13:49:09 +0100 Subject: [PATCH 21/25] pidfs: ensure that PIDFS_INFO_EXIT is available When we currently create a pidfd we check that the task hasn't been reaped right before we create the pidfd. But it is of course possible that by the time we return the pidfd to userspace the task has already been reaped since we don't check again after having created a dentry for it. This was fine until now because that race was meaningless. But now that we provide PIDFD_INFO_EXIT it is a problem because it is possible that the kernel returns a reaped pidfd and it depends on the race whether PIDFD_INFO_EXIT information is available. This depends on if the task gets reaped before or after a dentry has been attached to struct pid. Make this consistent and only returned pidfds for reaped tasks if PIDFD_INFO_EXIT information is available. This is done by performing another check whether the task has been reaped right after we attached a dentry to struct pid. Since pidfs_exit() is called before struct pid's task linkage is removed the case where the task got reaped but a dentry was already attached to struct pid and exit information was recorded and published can be handled correctly. In that case we do return a pidfd for a reaped task like we would've before. Link: https://lore.kernel.org/r/20250316-kabel-fehden-66bdb6a83436@brauner Reviewed-by: Oleg Nesterov Signed-off-by: Christian Brauner --- fs/pidfs.c | 56 ++++++++++++++++++++++++++++++++++++-- include/uapi/linux/pidfd.h | 4 +++ kernel/fork.c | 7 +++-- 3 files changed, 62 insertions(+), 5 deletions(-) diff --git a/fs/pidfs.c b/fs/pidfs.c index 3c630e9d4a62..a48cc44ced6f 100644 --- a/fs/pidfs.c +++ b/fs/pidfs.c @@ -753,8 +753,49 @@ static int pidfs_export_permission(struct handle_to_path_ctx *ctx, return 0; } +static inline bool pidfs_pid_valid(struct pid *pid, const struct path *path, + unsigned int flags) +{ + enum pid_type type; + + if (flags & PIDFD_CLONE) + return true; + + /* + * Make sure that if a pidfd is created PIDFD_INFO_EXIT + * information will be available. So after an inode for the + * pidfd has been allocated perform another check that the pid + * is still alive. If it is exit information is available even + * if the task gets reaped before the pidfd is returned to + * userspace. The only exception is PIDFD_CLONE where no task + * linkage has been established for @pid yet and the kernel is + * in the middle of process creation so there's nothing for + * pidfs to miss. + */ + if (flags & PIDFD_THREAD) + type = PIDTYPE_PID; + else + type = PIDTYPE_TGID; + + /* + * Since pidfs_exit() is called before struct pid's task linkage + * is removed the case where the task got reaped but a dentry + * was already attached to struct pid and exit information was + * recorded and published can be handled correctly. + */ + if (unlikely(!pid_has_task(pid, type))) { + struct inode *inode = d_inode(path->dentry); + return !!READ_ONCE(pidfs_i(inode)->exit_info); + } + + return true; +} + static struct file *pidfs_export_open(struct path *path, unsigned int oflags) { + if (!pidfs_pid_valid(d_inode(path->dentry)->i_private, path, oflags)) + return ERR_PTR(-ESRCH); + /* * Clear O_LARGEFILE as open_by_handle_at() forces it and raise * O_RDWR as pidfds always are. @@ -818,21 +859,30 @@ static struct file_system_type pidfs_type = { struct file *pidfs_alloc_file(struct pid *pid, unsigned int flags) { - struct file *pidfd_file; - struct path path; + struct path path __free(path_put) = {}; int ret; + /* + * Ensure that PIDFD_CLONE can be passed as a flag without + * overloading other uapi pidfd flags. + */ + BUILD_BUG_ON(PIDFD_CLONE == PIDFD_THREAD); + BUILD_BUG_ON(PIDFD_CLONE == PIDFD_NONBLOCK); + ret = path_from_stashed(&pid->stashed, pidfs_mnt, get_pid(pid), &path); if (ret < 0) return ERR_PTR(ret); + if (!pidfs_pid_valid(pid, &path, flags)) + return ERR_PTR(-ESRCH); + + flags &= ~PIDFD_CLONE; pidfd_file = dentry_open(&path, flags, current_cred()); /* Raise PIDFD_THREAD explicitly as do_dentry_open() strips it. */ if (!IS_ERR(pidfd_file)) pidfd_file->f_flags |= (flags & PIDFD_THREAD); - path_put(&path); return pidfd_file; } diff --git a/include/uapi/linux/pidfd.h b/include/uapi/linux/pidfd.h index 5cd5dcbfe884..2970ef44655a 100644 --- a/include/uapi/linux/pidfd.h +++ b/include/uapi/linux/pidfd.h @@ -10,6 +10,10 @@ /* Flags for pidfd_open(). */ #define PIDFD_NONBLOCK O_NONBLOCK #define PIDFD_THREAD O_EXCL +#ifdef __KERNEL__ +#include +#define PIDFD_CLONE CLONE_PIDFD +#endif /* Flags for pidfd_send_signal(). */ #define PIDFD_SIGNAL_THREAD (1UL << 0) diff --git a/kernel/fork.c b/kernel/fork.c index 8eac9cd3385b..f11ac96b7587 100644 --- a/kernel/fork.c +++ b/kernel/fork.c @@ -2425,8 +2425,11 @@ __latent_entropy struct task_struct *copy_process( if (clone_flags & CLONE_PIDFD) { int flags = (clone_flags & CLONE_THREAD) ? PIDFD_THREAD : 0; - /* Note that no task has been attached to @pid yet. */ - retval = __pidfd_prepare(pid, flags, &pidfile); + /* + * Note that no task has been attached to @pid yet indicate + * that via CLONE_PIDFD. + */ + retval = __pidfd_prepare(pid, flags | PIDFD_CLONE, &pidfile); if (retval < 0) goto bad_fork_free_pid; pidfd = retval; From 0fb482728ba1ee2130eaa461bf551f014447997c Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Thu, 20 Mar 2025 14:24:08 +0100 Subject: [PATCH 22/25] pidfs: improve multi-threaded exec and premature thread-group leader exit polling This is another attempt trying to make pidfd polling for multi-threaded exec and premature thread-group leader exit consistent. A quick recap of these two cases: (1) During a multi-threaded exec by a subthread, i.e., non-thread-group leader thread, all other threads in the thread-group including the thread-group leader are killed and the struct pid of the thread-group leader will be taken over by the subthread that called exec. IOW, two tasks change their TIDs. (2) A premature thread-group leader exit means that the thread-group leader exited before all of the other subthreads in the thread-group have exited. Both cases lead to inconsistencies for pidfd polling with PIDFD_THREAD. Any caller that holds a PIDFD_THREAD pidfd to the current thread-group leader may or may not see an exit notification on the file descriptor depending on when poll is performed. If the poll is performed before the exec of the subthread has concluded an exit notification is generated for the old thread-group leader. If the poll is performed after the exec of the subthread has concluded no exit notification is generated for the old thread-group leader. The correct behavior would be to simply not generate an exit notification on the struct pid of a subhthread exec because the struct pid is taken over by the subthread and thus remains alive. But this is difficult to handle because a thread-group may exit prematurely as mentioned in (2). In that case an exit notification is reliably generated but the subthreads may continue to run for an indeterminate amount of time and thus also may exec at some point. So far there was no way to distinguish between (1) and (2) internally. This tiny series tries to address this problem by discarding PIDFD_THREAD notification on premature thread-group leader exit. If that works correctly then no exit notifications are generated for a PIDFD_THREAD pidfd for a thread-group leader until all subthreads have been reaped. If a subthread should exec aftewards no exit notification will be generated until that task exits or it creates subthreads and repeates the cycle. Co-Developed-by: Oleg Nesterov Signed-off-by: Oleg Nesterov Link: https://lore.kernel.org/r/20250320-work-pidfs-thread_group-v4-1-da678ce805bf@kernel.org Signed-off-by: Christian Brauner --- fs/pidfs.c | 9 +++++---- kernel/exit.c | 6 +++--- kernel/signal.c | 3 +-- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/fs/pidfs.c b/fs/pidfs.c index a48cc44ced6f..1b3d23e0ffdd 100644 --- a/fs/pidfs.c +++ b/fs/pidfs.c @@ -210,20 +210,21 @@ static void pidfd_show_fdinfo(struct seq_file *m, struct file *f) static __poll_t pidfd_poll(struct file *file, struct poll_table_struct *pts) { struct pid *pid = pidfd_pid(file); - bool thread = file->f_flags & PIDFD_THREAD; struct task_struct *task; __poll_t poll_flags = 0; poll_wait(file, &pid->wait_pidfd, pts); /* - * Depending on PIDFD_THREAD, inform pollers when the thread - * or the whole thread-group exits. + * Don't wake waiters if the thread-group leader exited + * prematurely. They either get notified when the last subthread + * exits or not at all if one of the remaining subthreads execs + * and assumes the struct pid of the old thread-group leader. */ guard(rcu)(); task = pid_task(pid, PIDTYPE_PID); if (!task) poll_flags = EPOLLIN | EPOLLRDNORM | EPOLLHUP; - else if (task->exit_state && (thread || thread_group_empty(task))) + else if (task->exit_state && !delay_group_leader(task)) poll_flags = EPOLLIN | EPOLLRDNORM; return poll_flags; diff --git a/kernel/exit.c b/kernel/exit.c index 9916305e34d3..683766316a3d 100644 --- a/kernel/exit.c +++ b/kernel/exit.c @@ -743,10 +743,10 @@ static void exit_notify(struct task_struct *tsk, int group_dead) tsk->exit_state = EXIT_ZOMBIE; /* - * sub-thread or delay_group_leader(), wake up the - * PIDFD_THREAD waiters. + * Ignore thread-group leaders that exited before all + * subthreads did. */ - if (!thread_group_empty(tsk)) + if (!delay_group_leader(tsk)) do_notify_pidfd(tsk); if (unlikely(tsk->ptrace)) { diff --git a/kernel/signal.c b/kernel/signal.c index 081f19a24506..027ad9e97417 100644 --- a/kernel/signal.c +++ b/kernel/signal.c @@ -2180,8 +2180,7 @@ bool do_notify_parent(struct task_struct *tsk, int sig) WARN_ON_ONCE(!tsk->ptrace && (tsk->group_leader != tsk || !thread_group_empty(tsk))); /* - * tsk is a group leader and has no threads, wake up the - * non-PIDFD_THREAD waiters. + * Notify for thread-group leaders without subthreads. */ if (thread_group_empty(tsk)) do_notify_pidfd(tsk); From db7ce91e226df0fec200bd2a0eb87eed7c5d3adb Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Thu, 20 Mar 2025 14:24:09 +0100 Subject: [PATCH 23/25] selftests/pidfd: first test for multi-threaded exec polling Add first test for premature thread-group leader exit. Link: https://lore.kernel.org/r/20250320-work-pidfs-thread_group-v4-2-da678ce805bf@kernel.org Signed-off-by: Christian Brauner --- .../testing/selftests/pidfd/pidfd_info_test.c | 38 +++++++++++++++---- 1 file changed, 31 insertions(+), 7 deletions(-) diff --git a/tools/testing/selftests/pidfd/pidfd_info_test.c b/tools/testing/selftests/pidfd/pidfd_info_test.c index 09bc4ae7aed5..28a28ae4686a 100644 --- a/tools/testing/selftests/pidfd/pidfd_info_test.c +++ b/tools/testing/selftests/pidfd/pidfd_info_test.c @@ -236,7 +236,7 @@ static void *pidfd_info_pause_thread(void *arg) TEST_F(pidfd_info, thread_group) { - pid_t pid_leader, pid_thread; + pid_t pid_leader, pid_poller, pid_thread; pthread_t thread; int nevents, pidfd_leader, pidfd_thread, pidfd_leader_thread, ret; int ipc_sockets[2]; @@ -262,6 +262,35 @@ TEST_F(pidfd_info, thread_group) syscall(__NR_exit, EXIT_SUCCESS); } + /* + * Opening a PIDFD_THREAD aka thread-specific pidfd based on a + * thread-group leader must succeed. + */ + pidfd_leader_thread = sys_pidfd_open(pid_leader, PIDFD_THREAD); + ASSERT_GE(pidfd_leader_thread, 0); + + pid_poller = fork(); + ASSERT_GE(pid_poller, 0); + if (pid_poller == 0) { + /* + * We can't poll and wait for the old thread-group + * leader to exit using a thread-specific pidfd. The + * thread-group leader exited prematurely and + * notification is delayed until all subthreads have + * exited. + */ + fds.events = POLLIN; + fds.fd = pidfd_leader_thread; + nevents = poll(&fds, 1, 10000 /* wait 5 seconds */); + if (nevents != 0) + _exit(EXIT_FAILURE); + if (fds.revents & POLLIN) + _exit(EXIT_FAILURE); + if (fds.revents & POLLHUP) + _exit(EXIT_FAILURE); + _exit(EXIT_SUCCESS); + } + /* Retrieve the tid of the thread. */ EXPECT_EQ(close(ipc_sockets[1]), 0); ASSERT_EQ(read_nointr(ipc_sockets[0], &pid_thread, sizeof(pid_thread)), sizeof(pid_thread)); @@ -275,12 +304,7 @@ TEST_F(pidfd_info, thread_group) pidfd_thread = sys_pidfd_open(pid_thread, PIDFD_THREAD); ASSERT_GE(pidfd_thread, 0); - /* - * Opening a PIDFD_THREAD aka thread-specific pidfd based on a - * thread-group leader must succeed. - */ - pidfd_leader_thread = sys_pidfd_open(pid_leader, PIDFD_THREAD); - ASSERT_GE(pidfd_leader_thread, 0); + ASSERT_EQ(wait_for_pid(pid_poller), 0); /* * Note that pidfd_leader is a thread-group pidfd, so polling on it From 9b6f723db5365b995f39c1f4491a0004cc490dd6 Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Thu, 20 Mar 2025 14:24:10 +0100 Subject: [PATCH 24/25] selftests/pidfd: second test for multi-threaded exec polling Ensure that during a multi-threaded exec and premature thread-group leader exit no exit notification is generated. Link: https://lore.kernel.org/r/20250320-work-pidfs-thread_group-v4-3-da678ce805bf@kernel.org Signed-off-by: Christian Brauner --- .../testing/selftests/pidfd/pidfd_info_test.c | 72 ++++++++++++------- 1 file changed, 48 insertions(+), 24 deletions(-) diff --git a/tools/testing/selftests/pidfd/pidfd_info_test.c b/tools/testing/selftests/pidfd/pidfd_info_test.c index 28a28ae4686a..4169780c9e55 100644 --- a/tools/testing/selftests/pidfd/pidfd_info_test.c +++ b/tools/testing/selftests/pidfd/pidfd_info_test.c @@ -413,7 +413,7 @@ static void *pidfd_info_thread_exec(void *arg) TEST_F(pidfd_info, thread_group_exec) { - pid_t pid_leader, pid_thread; + pid_t pid_leader, pid_poller, pid_thread; pthread_t thread; int nevents, pidfd_leader, pidfd_leader_thread, pidfd_thread, ret; int ipc_sockets[2]; @@ -439,6 +439,37 @@ TEST_F(pidfd_info, thread_group_exec) syscall(__NR_exit, EXIT_SUCCESS); } + /* Open a thread-specific pidfd for the thread-group leader. */ + pidfd_leader_thread = sys_pidfd_open(pid_leader, PIDFD_THREAD); + ASSERT_GE(pidfd_leader_thread, 0); + + pid_poller = fork(); + ASSERT_GE(pid_poller, 0); + if (pid_poller == 0) { + /* + * We can't poll and wait for the old thread-group + * leader to exit using a thread-specific pidfd. The + * thread-group leader exited prematurely and + * notification is delayed until all subthreads have + * exited. + * + * When the thread has execed it will taken over the old + * thread-group leaders struct pid. Calling poll after + * the thread execed will thus block again because a new + * thread-group has started. + */ + fds.events = POLLIN; + fds.fd = pidfd_leader_thread; + nevents = poll(&fds, 1, 10000 /* wait 5 seconds */); + if (nevents != 0) + _exit(EXIT_FAILURE); + if (fds.revents & POLLIN) + _exit(EXIT_FAILURE); + if (fds.revents & POLLHUP) + _exit(EXIT_FAILURE); + _exit(EXIT_SUCCESS); + } + /* Retrieve the tid of the thread. */ EXPECT_EQ(close(ipc_sockets[1]), 0); ASSERT_EQ(read_nointr(ipc_sockets[0], &pid_thread, sizeof(pid_thread)), sizeof(pid_thread)); @@ -447,33 +478,12 @@ TEST_F(pidfd_info, thread_group_exec) pidfd_thread = sys_pidfd_open(pid_thread, PIDFD_THREAD); ASSERT_GE(pidfd_thread, 0); - /* Open a thread-specific pidfd for the thread-group leader. */ - pidfd_leader_thread = sys_pidfd_open(pid_leader, PIDFD_THREAD); - ASSERT_GE(pidfd_leader_thread, 0); - - /* - * We can poll and wait for the old thread-group leader to exit - * using a thread-specific pidfd. - * - * This only works until the thread has execed. When the thread - * has execed it will have taken over the old thread-group - * leaders struct pid. Calling poll after the thread execed will - * thus block again because a new thread-group has started (Yes, - * it's fscked.). - */ - fds.events = POLLIN; - fds.fd = pidfd_leader_thread; - nevents = poll(&fds, 1, -1); - ASSERT_EQ(nevents, 1); - /* The thread-group leader has exited. */ - ASSERT_TRUE(!!(fds.revents & POLLIN)); - /* The thread-group leader hasn't been reaped. */ - ASSERT_FALSE(!!(fds.revents & POLLHUP)); - /* Now that we've opened a thread-specific pidfd the thread can exec. */ ASSERT_EQ(write_nointr(ipc_sockets[0], &pid_thread, sizeof(pid_thread)), sizeof(pid_thread)); EXPECT_EQ(close(ipc_sockets[0]), 0); + ASSERT_EQ(wait_for_pid(pid_poller), 0); + /* Wait until the kernel has SIGKILLed the thread. */ fds.events = POLLHUP; fds.fd = pidfd_thread; @@ -506,6 +516,20 @@ TEST_F(pidfd_info, thread_group_exec) /* Take down the thread-group leader. */ EXPECT_EQ(sys_pidfd_send_signal(pidfd_leader, SIGKILL, NULL, 0), 0); + + /* + * Afte the exec we're dealing with an empty thread-group so now + * we must see an exit notification on the thread-specific pidfd + * for the thread-group leader as there's no subthread that can + * revive the struct pid. + */ + fds.events = POLLIN; + fds.fd = pidfd_leader_thread; + nevents = poll(&fds, 1, -1); + ASSERT_EQ(nevents, 1); + ASSERT_TRUE(!!(fds.revents & POLLIN)); + ASSERT_FALSE(!!(fds.revents & POLLHUP)); + EXPECT_EQ(sys_waitid(P_PIDFD, pidfd_leader, NULL, WEXITED), 0); /* Retrieve exit information for the thread-group leader. */ From 4d5483a42c6f7c5268ddcdfac9f4f35424e1f186 Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Thu, 20 Mar 2025 14:24:11 +0100 Subject: [PATCH 25/25] selftests/pidfd: third test for multi-threaded exec polling Ensure that during a multi-threaded exec and premature thread-group leader exit no exit notification is generated. Link: https://lore.kernel.org/r/20250320-work-pidfs-thread_group-v4-4-da678ce805bf@kernel.org Signed-off-by: Christian Brauner --- .../testing/selftests/pidfd/pidfd_info_test.c | 147 ++++++++++++++++++ 1 file changed, 147 insertions(+) diff --git a/tools/testing/selftests/pidfd/pidfd_info_test.c b/tools/testing/selftests/pidfd/pidfd_info_test.c index 4169780c9e55..1758a1b0457b 100644 --- a/tools/testing/selftests/pidfd/pidfd_info_test.c +++ b/tools/testing/selftests/pidfd/pidfd_info_test.c @@ -542,4 +542,151 @@ TEST_F(pidfd_info, thread_group_exec) EXPECT_EQ(close(pidfd_thread), 0); } +static void *pidfd_info_thread_exec_sane(void *arg) +{ + pid_t pid_thread = gettid(); + int ipc_socket = *(int *)arg; + + /* Inform the grand-parent what the tid of this thread is. */ + if (write_nointr(ipc_socket, &pid_thread, sizeof(pid_thread)) != sizeof(pid_thread)) + return NULL; + + if (read_nointr(ipc_socket, &pid_thread, sizeof(pid_thread)) != sizeof(pid_thread)) + return NULL; + + close(ipc_socket); + + sys_execveat(AT_FDCWD, "pidfd_exec_helper", NULL, NULL, 0); + return NULL; +} + +TEST_F(pidfd_info, thread_group_exec_thread) +{ + pid_t pid_leader, pid_poller, pid_thread; + pthread_t thread; + int nevents, pidfd_leader, pidfd_leader_thread, pidfd_thread, ret; + int ipc_sockets[2]; + struct pollfd fds = {}; + struct pidfd_info info = { + .mask = PIDFD_INFO_CGROUPID | PIDFD_INFO_EXIT, + }; + + ret = socketpair(AF_LOCAL, SOCK_STREAM | SOCK_CLOEXEC, 0, ipc_sockets); + EXPECT_EQ(ret, 0); + + pid_leader = create_child(&pidfd_leader, 0); + EXPECT_GE(pid_leader, 0); + + if (pid_leader == 0) { + close(ipc_sockets[0]); + + /* The thread will outlive the thread-group leader. */ + if (pthread_create(&thread, NULL, pidfd_info_thread_exec_sane, &ipc_sockets[1])) + syscall(__NR_exit, EXIT_FAILURE); + + /* + * Pause the thread-group leader. It will be killed once + * the subthread execs. + */ + pause(); + syscall(__NR_exit, EXIT_SUCCESS); + } + + /* Retrieve the tid of the thread. */ + EXPECT_EQ(close(ipc_sockets[1]), 0); + ASSERT_EQ(read_nointr(ipc_sockets[0], &pid_thread, sizeof(pid_thread)), sizeof(pid_thread)); + + /* Opening a thread as a PIDFD_THREAD must succeed. */ + pidfd_thread = sys_pidfd_open(pid_thread, PIDFD_THREAD); + ASSERT_GE(pidfd_thread, 0); + + /* Open a thread-specific pidfd for the thread-group leader. */ + pidfd_leader_thread = sys_pidfd_open(pid_leader, PIDFD_THREAD); + ASSERT_GE(pidfd_leader_thread, 0); + + pid_poller = fork(); + ASSERT_GE(pid_poller, 0); + if (pid_poller == 0) { + /* + * The subthread will now exec. The struct pid of the old + * thread-group leader will be assumed by the subthread which + * becomes the new thread-group leader. So no exit notification + * must be generated. Wait for 5 seconds and call it a success + * if no notification has been received. + */ + fds.events = POLLIN; + fds.fd = pidfd_leader_thread; + nevents = poll(&fds, 1, 10000 /* wait 5 seconds */); + if (nevents != 0) + _exit(EXIT_FAILURE); + if (fds.revents & POLLIN) + _exit(EXIT_FAILURE); + if (fds.revents & POLLHUP) + _exit(EXIT_FAILURE); + _exit(EXIT_SUCCESS); + } + + /* Now that we've opened a thread-specific pidfd the thread can exec. */ + ASSERT_EQ(write_nointr(ipc_sockets[0], &pid_thread, sizeof(pid_thread)), sizeof(pid_thread)); + EXPECT_EQ(close(ipc_sockets[0]), 0); + ASSERT_EQ(wait_for_pid(pid_poller), 0); + + /* Wait until the kernel has SIGKILLed the thread. */ + fds.events = POLLHUP; + fds.fd = pidfd_thread; + nevents = poll(&fds, 1, -1); + ASSERT_EQ(nevents, 1); + /* The thread has been reaped. */ + ASSERT_TRUE(!!(fds.revents & POLLHUP)); + + /* Retrieve thread-specific exit info from pidfd. */ + ASSERT_EQ(ioctl(pidfd_thread, PIDFD_GET_INFO, &info), 0); + ASSERT_FALSE(!!(info.mask & PIDFD_INFO_CREDS)); + ASSERT_TRUE(!!(info.mask & PIDFD_INFO_EXIT)); + /* + * While the kernel will have SIGKILLed the whole thread-group + * during exec it will cause the individual threads to exit + * cleanly. + */ + ASSERT_TRUE(WIFEXITED(info.exit_code)); + ASSERT_EQ(WEXITSTATUS(info.exit_code), 0); + + /* + * The thread-group leader is still alive, the thread has taken + * over its struct pid and thus its pid number. + */ + info.mask = PIDFD_INFO_CGROUPID | PIDFD_INFO_EXIT; + ASSERT_EQ(ioctl(pidfd_leader, PIDFD_GET_INFO, &info), 0); + ASSERT_TRUE(!!(info.mask & PIDFD_INFO_CREDS)); + ASSERT_FALSE(!!(info.mask & PIDFD_INFO_EXIT)); + ASSERT_EQ(info.pid, pid_leader); + + /* Take down the thread-group leader. */ + EXPECT_EQ(sys_pidfd_send_signal(pidfd_leader, SIGKILL, NULL, 0), 0); + + /* + * Afte the exec we're dealing with an empty thread-group so now + * we must see an exit notification on the thread-specific pidfd + * for the thread-group leader as there's no subthread that can + * revive the struct pid. + */ + fds.events = POLLIN; + fds.fd = pidfd_leader_thread; + nevents = poll(&fds, 1, -1); + ASSERT_EQ(nevents, 1); + ASSERT_TRUE(!!(fds.revents & POLLIN)); + ASSERT_FALSE(!!(fds.revents & POLLHUP)); + + EXPECT_EQ(sys_waitid(P_PIDFD, pidfd_leader, NULL, WEXITED), 0); + + /* Retrieve exit information for the thread-group leader. */ + info.mask = PIDFD_INFO_CGROUPID | PIDFD_INFO_EXIT; + ASSERT_EQ(ioctl(pidfd_leader, PIDFD_GET_INFO, &info), 0); + ASSERT_FALSE(!!(info.mask & PIDFD_INFO_CREDS)); + ASSERT_TRUE(!!(info.mask & PIDFD_INFO_EXIT)); + + EXPECT_EQ(close(pidfd_leader), 0); + EXPECT_EQ(close(pidfd_thread), 0); +} + TEST_HARNESS_MAIN