diff options
author | 2025-04-04 16:06:55 +0200 | |
---|---|---|
committer | 2025-04-07 09:38:24 +0200 | |
commit | 1e940fff94374d04b6c34f896ed9fbad3d2fb706 (patch) | |
tree | b8b36d1b847d960dfb7bb9d76b4e477b65ebd656 | |
parent | Linux 6.15-rc1 (diff) | |
parent | selftest/pidfd: add test for thread-group leader pidfd open for thread (diff) | |
download | wireguard-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.c | 23 | ||||
-rw-r--r-- | tools/testing/selftests/pidfd/pidfd_info_test.c | 13 |
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; |