From: Tycho Andersen <tycho@tycho.pizza>
To: Christian Brauner <brauner@kernel.org>
Cc: Oleg Nesterov <oleg@redhat.com>,
"Eric W . Biederman" <ebiederm@xmission.com>,
linux-kernel@vger.kernel.org, linux-api@vger.kernel.org,
Tycho Andersen <tandersen@netflix.com>, Jan Kara <jack@suse.cz>,
linux-fsdevel@vger.kernel.org,
Joel Fernandes <joel@joelfernandes.org>
Subject: Re: [RFC 1/3] pidfd: allow pidfd_open() on non-thread-group leaders
Date: Fri, 8 Dec 2023 13:04:11 -0700 [thread overview]
Message-ID: <ZXN2u2oJl1Z6FTqt@tycho.pizza> (raw)
In-Reply-To: <20231207-avancieren-unbezahlbar-9258f45ec3ec@brauner>
On Thu, Dec 07, 2023 at 10:25:09PM +0100, Christian Brauner wrote:
> > If these concerns are correct
>
> So, ok. I misremebered this. The scenario I had been thinking of is
> basically the following.
>
> We have a thread-group with thread-group leader 1234 and a thread with
> 4567 in that thread-group. Assume current thread-group leader is tsk1
> and the non-thread-group leader is tsk2. tsk1 uses struct pid *tg_pid
> and tsk2 uses struct pid *t_pid. The struct pids look like this after
> creation of both thread-group leader tsk1 and thread tsk2:
>
> TGID 1234 TID 4567
> tg_pid[PIDTYPE_PID] = tsk1 t_pid[PIDTYPE_PID] = tsk2
> tg_pid[PIDTYPE_TGID] = tsk1 t_pid[PIDTYPE_TGID] = NULL
>
> IOW, tsk2's struct pid has never been used as a thread-group leader and
> thus PIDTYPE_TGID is NULL. Now assume someone does create pidfds for
> tsk1 and for tsk2:
>
> tg_pidfd = pidfd_open(tsk1) t_pidfd = pidfd_open(tsk2)
> -> tg_pidfd->private_data = tg_pid -> t_pidfd->private_data = t_pid
>
> So we stash away struct pid *tg_pid for a pidfd_open() on tsk1 and we
> stash away struct pid *t_pid for a pidfd_open() on tsk2.
>
> If we wait on that task via P_PIDFD we get:
>
> /* waiting through pidfd */
> waitid(P_PIDFD, tg_pidfd) waitid(P_PIDFD, t_pidfd)
> tg_pid[PIDTYPE_TGID] == tsk1 t_pid[PIDTYPE_TGID] == NULL
> => succeeds => fails
>
> Because struct pid *tg_pid is used a thread-group leader struct pid we
> can wait on that tsk1. But we can't via the non-thread-group leader
> pidfd because the struct pid *t_pid has never been used as a
> thread-group leader.
>
> Now assume, t_pid exec's and the struct pids are transfered. IIRC, we
> get:
>
> tg_pid[PIDTYPE_PID] = tsk2 t_pid[PIDTYPE_PID] = tsk1
> tg_pid[PIDTYPE_TGID] = tsk2 t_pid[PIDTYPE_TGID] = NULL
>
> If we wait on that task via P_PIDFD we get:
>
> /* waiting through pidfd */
> waitid(P_PIDFD, tg_pidfd) waitid(P_PIDFD, t_pid)
> tg_pid[PIDTYPE_TGID] == tsk2 t_pid[PIDTYPE_TGID] == NULL
> => succeeds => fails
>
> Which is what we want. So effectively this should all work and I
> misremembered the struct pid linkage. So afaict we don't even have a
> problem here which is great.
It sounds like we need some tests for waitpid() directly though, to
ensure the semantics stay stable. I can add those and send a v3,
assuming the location of do_notify_pidfd() looks ok to you in v2:
https://lore.kernel.org/all/20231207170946.130823-1-tycho@tycho.pizza/
Tycho
next prev parent reply other threads:[~2023-12-08 20:04 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-11-30 16:39 [RFC 1/3] pidfd: allow pidfd_open() on non-thread-group leaders Tycho Andersen
2023-11-30 16:39 ` [RFC 2/3] selftests/pidfd: add non-thread-group leader tests Tycho Andersen
2023-11-30 16:39 ` [RFC 3/3] clone: allow CLONE_THREAD | CLONE_PIDFD together Tycho Andersen
2023-11-30 17:39 ` [RFC 1/3] pidfd: allow pidfd_open() on non-thread-group leaders Oleg Nesterov
2023-11-30 17:56 ` Tycho Andersen
2023-12-01 16:31 ` Tycho Andersen
2023-12-07 17:57 ` Christian Brauner
2023-12-07 21:25 ` Christian Brauner
2023-12-08 20:04 ` Tycho Andersen [this message]
2023-11-30 18:37 ` Florian Weimer
2023-11-30 18:54 ` Tycho Andersen
2023-11-30 19:00 ` Mathieu Desnoyers
2023-11-30 19:17 ` Tycho Andersen
2023-11-30 19:43 ` Florian Weimer
2023-12-06 15:27 ` Tycho Andersen
2023-12-07 22:58 ` Christian Brauner
2023-12-08 3:16 ` Jens Axboe
2023-12-08 13:15 ` Florian Weimer
2023-12-08 13:48 ` Christian Brauner
2023-12-08 13:58 ` Florian Weimer
2023-12-07 17:21 ` Christian Brauner
2023-12-07 17:52 ` Tycho Andersen
2023-12-08 17:47 ` Jan Kara
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=ZXN2u2oJl1Z6FTqt@tycho.pizza \
--to=tycho@tycho.pizza \
--cc=brauner@kernel.org \
--cc=ebiederm@xmission.com \
--cc=jack@suse.cz \
--cc=joel@joelfernandes.org \
--cc=linux-api@vger.kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=oleg@redhat.com \
--cc=tandersen@netflix.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).