aboutsummaryrefslogtreecommitdiffstatshomepage
diff options
context:
space:
mode:
authorChristian Brauner <brauner@kernel.org>2025-04-04 16:06:55 +0200
committerChristian Brauner <brauner@kernel.org>2025-04-07 09:38:24 +0200
commit1e940fff94374d04b6c34f896ed9fbad3d2fb706 (patch)
treeb8b36d1b847d960dfb7bb9d76b4e477b65ebd656
parentLinux 6.15-rc1 (diff)
parentselftest/pidfd: add test for thread-group leader pidfd open for thread (diff)
downloadwireguard-linux-1e940fff94374d04b6c34f896ed9fbad3d2fb706.tar.xz
wireguard-linux-1e940fff94374d04b6c34f896ed9fbad3d2fb706.zip
Merge patch series "pidfd: improve uapi when task isn't found"
Christian Brauner <brauner@kernel.org> says: We currently report EINVAL whenever a struct pid has no tasked attached anymore thereby conflating two concepts: (1) The task has already been reaped. (2) The caller requested a pidfd for a thread-group leader but the pid actually references a struct pid that isn't used as a thread-group leader. This is causing issues for non-threaded workloads as in [1] where they expect ESRCH to be reported, not EINVAL. I think that's a very resonable assumption. This patch tries to allow userspace to distinguish between (1) and (2). This is racy of course but that shouldn't matter. * patches from https://lore.kernel.org/r/20250403-work-pidfd-fixes-v1-0-a123b6ed6716@kernel.org: selftest/pidfd: add test for thread-group leader pidfd open for thread pidfd: improve uapi when task isn't found pidfd: remove unneeded NULL check from pidfd_prepare() selftests/pidfd: adapt to recent changes Link: https://lore.kernel.org/r/20250403-work-pidfd-fixes-v1-0-a123b6ed6716@kernel.org Signed-off-by: Christian Brauner <brauner@kernel.org>
-rw-r--r--kernel/fork.c23
-rw-r--r--tools/testing/selftests/pidfd/pidfd_info_test.c13
2 files changed, 27 insertions, 9 deletions
diff --git a/kernel/fork.c b/kernel/fork.c
index c4b26cd8998b..4a2080b968c8 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -2108,10 +2108,27 @@ static int __pidfd_prepare(struct pid *pid, unsigned int flags, struct file **re
*/
int pidfd_prepare(struct pid *pid, unsigned int flags, struct file **ret)
{
- bool thread = flags & PIDFD_THREAD;
+ int err = 0;
- if (!pid || !pid_has_task(pid, thread ? PIDTYPE_PID : PIDTYPE_TGID))
- return -EINVAL;
+ if (!(flags & PIDFD_THREAD)) {
+ /*
+ * If this is struct pid isn't used as a thread-group
+ * leader pid but the caller requested to create a
+ * thread-group leader pidfd then report ENOENT to the
+ * caller as a hint.
+ */
+ if (!pid_has_task(pid, PIDTYPE_TGID))
+ err = -ENOENT;
+ }
+
+ /*
+ * If this wasn't a thread-group leader struct pid or the task
+ * got reaped in the meantime report -ESRCH to userspace.
+ */
+ if (!pid_has_task(pid, PIDTYPE_PID))
+ err = -ESRCH;
+ if (err)
+ return err;
return __pidfd_prepare(pid, flags, ret);
}
diff --git a/tools/testing/selftests/pidfd/pidfd_info_test.c b/tools/testing/selftests/pidfd/pidfd_info_test.c
index 1758a1b0457b..a0eb6e81eaa2 100644
--- a/tools/testing/selftests/pidfd/pidfd_info_test.c
+++ b/tools/testing/selftests/pidfd/pidfd_info_test.c
@@ -299,6 +299,7 @@ TEST_F(pidfd_info, thread_group)
/* Opening a thread as a thread-group leader must fail. */
pidfd_thread = sys_pidfd_open(pid_thread, 0);
ASSERT_LT(pidfd_thread, 0);
+ ASSERT_EQ(errno, ENOENT);
/* Opening a thread as a PIDFD_THREAD must succeed. */
pidfd_thread = sys_pidfd_open(pid_thread, PIDFD_THREAD);
@@ -362,9 +363,9 @@ TEST_F(pidfd_info, thread_group)
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);
+ /* Even though the thread-group exited successfully it will still report the group exit code. */
+ ASSERT_TRUE(WIFSIGNALED(info.exit_code));
+ ASSERT_EQ(WTERMSIG(info.exit_code), SIGKILL);
/*
* Retrieve exit information for the thread-group leader via the
@@ -375,9 +376,9 @@ TEST_F(pidfd_info, thread_group)
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);
+ /* Even though the thread-group exited successfully it will still report the group exit code. */
+ ASSERT_TRUE(WIFSIGNALED(info2.exit_code));
+ ASSERT_EQ(WTERMSIG(info2.exit_code), SIGKILL);
/* Retrieve exit information for the thread. */
info.mask = PIDFD_INFO_CGROUPID | PIDFD_INFO_EXIT;