Unverified Commit 1e940fff authored by Christian Brauner's avatar Christian Brauner
Browse files

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: default avatarChristian Brauner <brauner@kernel.org>
parents 0af2f6be 4fc3f73c

kernel/fork.c

+20 −3
Original line number Diff line number Diff line
@@ -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);

}

tools/testing/selftests/pidfd/pidfd_info_test.c

+7 −6
Original line number Diff line number Diff line
@@ -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;