From: Kuniyuki Iwashima <kuniyu@amazon.com>
To: <brauner@kernel.org>
Cc: <alexander@mihalicyn.com>, <bluca@debian.org>,
<daan.j.demeyer@gmail.com>, <davem@davemloft.net>,
<david@readahead.eu>, <edumazet@google.com>, <horms@kernel.org>,
<jack@suse.cz>, <kuba@kernel.org>, <kuniyu@amazon.com>,
<lennart@poettering.net>, <linux-fsdevel@vger.kernel.org>,
<linux-kernel@vger.kernel.org>, <me@yhndnzj.com>,
<netdev@vger.kernel.org>, <oleg@redhat.com>, <pabeni@redhat.com>
Subject: Re: [PATCH v2 2/4] net, pidfs: prepare for handing out pidfds for reaped sk->sk_peer_pid
Date: Fri, 25 Apr 2025 13:46:05 -0700 [thread overview]
Message-ID: <20250425204632.44889-1-kuniyu@amazon.com> (raw)
In-Reply-To: <20250425-work-pidfs-net-v2-2-450a19461e75@kernel.org>
From: Christian Brauner <brauner@kernel.org>
Date: Fri, 25 Apr 2025 10:11:31 +0200
> SO_PEERPIDFD currently doesn't support handing out pidfds if the
> sk->sk_peer_pid thread-group leader has already been reaped. In this
> case it currently returns EINVAL. Userspace still wants to get a pidfd
> for a reaped process to have a stable handle it can pass on.
> This is especially useful now that it is possible to retrieve exit
> information through a pidfd via the PIDFD_GET_INFO ioctl()'s
> PIDFD_INFO_EXIT flag.
>
> Another summary has been provided by David in [1]:
>
> > A pidfd can outlive the task it refers to, and thus user-space must
> > already be prepared that the task underlying a pidfd is gone at the time
> > they get their hands on the pidfd. For instance, resolving the pidfd to
> > a PID via the fdinfo must be prepared to read `-1`.
> >
> > Despite user-space knowing that a pidfd might be stale, several kernel
> > APIs currently add another layer that checks for this. In particular,
> > SO_PEERPIDFD returns `EINVAL` if the peer-task was already reaped,
> > but returns a stale pidfd if the task is reaped immediately after the
> > respective alive-check.
> >
> > This has the unfortunate effect that user-space now has two ways to
> > check for the exact same scenario: A syscall might return
> > EINVAL/ESRCH/... *or* the pidfd might be stale, even though there is no
> > particular reason to distinguish both cases. This also propagates
> > through user-space APIs, which pass on pidfds. They must be prepared to
> > pass on `-1` *or* the pidfd, because there is no guaranteed way to get a
> > stale pidfd from the kernel.
> > Userspace must already deal with a pidfd referring to a reaped task as
> > the task may exit and get reaped at any time will there are still many
> > pidfds referring to it.
>
> In order to allow handing out reaped pidfd SO_PEERPIDFD needs to ensure
> that PIDFD_INFO_EXIT information is available whenever a pidfd for a
> reaped task is created by PIDFD_INFO_EXIT. The uapi promises that reaped
> pidfds are only handed out if it is guaranteed that the caller sees the
> exit information:
>
> TEST_F(pidfd_info, success_reaped)
> {
> struct pidfd_info info = {
> .mask = PIDFD_INFO_CGROUPID | PIDFD_INFO_EXIT,
> };
>
> /*
> * Process has already been reaped and PIDFD_INFO_EXIT been set.
> * Verify that we can retrieve the exit status of the process.
> */
> 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);
> }
>
> To hand out pidfds for reaped processes we thus allocate a pidfs entry
> for the relevant sk->sk_peer_pid at the time the sk->sk_peer_pid is
> stashed and drop it when the socket is destroyed. This guarantees that
> exit information will always be recorded for the sk->sk_peer_pid task
> and we can hand out pidfds for reaped processes.
>
> Link: https://lore.kernel.org/lkml/20230807085203.819772-1-david@readahead.eu [1]
> Signed-off-by: Christian Brauner <brauner@kernel.org>
Reviewed-by: Kuniyuki Iwashima <kuniyu@amazon.com>
next prev parent reply other threads:[~2025-04-25 20:46 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-04-25 8:11 [PATCH v2 0/4] net, pidfs: enable handing out pidfds for reaped sk->sk_peer_pid Christian Brauner
2025-04-25 8:11 ` [PATCH v2 1/4] pidfs: register pid in pidfs Christian Brauner
2025-04-25 8:11 ` [PATCH v2 2/4] net, pidfs: prepare for handing out pidfds for reaped sk->sk_peer_pid Christian Brauner
2025-04-25 20:46 ` Kuniyuki Iwashima [this message]
2025-04-25 8:11 ` [PATCH v2 3/4] pidfs: get rid of __pidfd_prepare() Christian Brauner
2025-04-25 8:11 ` [PATCH v2 4/4] net, pidfs: enable handing out pidfds for reaped sk->sk_peer_pid Christian Brauner
2025-04-25 20:47 ` Kuniyuki Iwashima
2025-04-25 8:38 ` [PATCH v2 0/4] " David Rheinsberg
2025-04-26 6:28 ` Christian Brauner
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250425204632.44889-1-kuniyu@amazon.com \
--to=kuniyu@amazon.com \
--cc=alexander@mihalicyn.com \
--cc=bluca@debian.org \
--cc=brauner@kernel.org \
--cc=daan.j.demeyer@gmail.com \
--cc=davem@davemloft.net \
--cc=david@readahead.eu \
--cc=edumazet@google.com \
--cc=horms@kernel.org \
--cc=jack@suse.cz \
--cc=kuba@kernel.org \
--cc=lennart@poettering.net \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=me@yhndnzj.com \
--cc=netdev@vger.kernel.org \
--cc=oleg@redhat.com \
--cc=pabeni@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.