linux-man.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Christian Brauner <christian.brauner@ubuntu.com>
To: "Michael Kerrisk (man-pages)" <mtk.manpages@gmail.com>
Cc: Florian Weimer <fw@deneb.enyo.de>,
	Oleg Nesterov <oleg@redhat.com>, Jann Horn <jannh@google.com>,
	"Eric W. Biederman" <ebiederm@xmission.com>,
	Daniel Colascione <dancol@google.com>,
	Joel Fernandes <joel@joelfernandes.org>,
	linux-man <linux-man@vger.kernel.org>,
	Linux API <linux-api@vger.kernel.org>,
	lkml <linux-kernel@vger.kernel.org>
Subject: Re: For review: pidfd_send_signal(2) manual page
Date: Tue, 24 Sep 2019 23:53:17 +0200	[thread overview]
Message-ID: <20190924215316.gxev2anuqffegocw@wittgenstein> (raw)
In-Reply-To: <b76adb4c-826b-6493-ba75-a9863066d3b1@gmail.com>

On Tue, Sep 24, 2019 at 11:00:03PM +0200, Michael Kerrisk (man-pages) wrote:
> Hello Christian,
> 
> >>> If you're the parent of the process you can do this without CLONE_PIDFD:
> >>> pid = fork();
> >>> pidfd = pidfd_open();
> >>> ret = pidfd_send_signal(pidfd, 0, NULL, 0);
> >>> if (ret < 0 && errno == ESRCH)
> >>> 	/* pidfd refers to another, recycled process */
> >>
> >> Although there is still the race between the fork() and the
> >> pidfd_open(), right?
> > 
> > Actually no and my code is even too complex.
> > If you are the parent, and this is really a sequence that obeys the
> > ordering pidfd_open() before waiting:
> > 
> > pid = fork();
> > if (pid == 0)
> > 	exit(EXIT_SUCCESS);
> > pidfd = pidfd_open(pid, 0);
> > waitid(pid, ...);
> > 
> > Then you are guaranteed that pidfd will refer to pid. No recycling can
> > happen since the process has not been waited upon yet (That is,
> 
> D'oh! Yes, of course. 
> 
> > excluding special cases such as where you have a mainloop where a
> > callback reacts to a SIGCHLD event and waits on the child behind your
> > back and your next callback in the mainloop calls pidfd_open() while the
> > pid has been recycled etc.).
> > A race could only appear in sequences where waiting happens before
> > pidfd_open():
> > 
> > pid = fork();
> > if (pid == 0)
> > 	exit(EXIT_SUCCESS);
> > waitid(pid, ...);
> > pidfd = pidfd_open(pid, 0);
> > 
> > which honestly simply doesn't make any sense. So if you're the parent
> > and you combine fork() + pidfd_open() correctly things should be fine
> > without even having to verify via pidfd_send_signal() (I missed that in
> > my first mail.).
> 
> Thanks for the additional detail.

You're very welcome.

> 
> I added the following to the pidfd_open() page, to
> prevent people making the same thinko as me:
> 
>        The following code sequence can be used to obtain a file  descrip‐
>        tor for the child of fork(2):
> 
>            pid = fork();
>            if (pid > 0) {     /* If parent */
>                pidfd = pidfd_open(pid, 0);
>                ...
>            }
> 
>        Even  if  the  child process has already terminated by the time of
>        the pidfd_open() call, the returned file descriptor is  guaranteed
>        to refer to the child because the parent has not yet waited on the
>        child (and therefore, the child's ID has not been recycled).

Thanks! I'm fine with the example. The code illustrates the basics. If
you want to go overboard, you can mention my callback example and put my
SIG_IGN code snippet from my earlier mails (cf. [1] and [2]) in there.
But imho, that'll complicate the manpage and I'm not sure it's worth it.

Thanks!
Christian

/* References */
[1]: https://lore.kernel.org/r/20190924195701.7pw2olbviieqsg5q@wittgenstein
[2]: https://lore.kernel.org/r/20190924200735.2dvqhan7ynnmfc7s@wittgenstein

  parent reply	other threads:[~2019-09-24 21:53 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-23  9:12 For review: pidfd_send_signal(2) manual page Michael Kerrisk (man-pages)
2019-09-23 11:26 ` Florian Weimer
2019-09-23 14:23   ` Christian Brauner
2019-09-24 19:44     ` Michael Kerrisk (man-pages)
2019-09-24 19:57       ` Christian Brauner
2019-09-24 20:07         ` Christian Brauner
2019-09-24 21:00         ` Michael Kerrisk (man-pages)
2019-09-24 21:08           ` Daniel Colascione
2019-09-25 13:46             ` Michael Kerrisk (man-pages)
2019-09-24 21:53           ` Christian Brauner [this message]
2019-09-25 13:46             ` Michael Kerrisk (man-pages)
2019-09-25 13:51               ` Florian Weimer
2019-09-25 14:02                 ` Michael Kerrisk (man-pages)
2019-09-25 13:53               ` Christian Brauner
2019-09-25 14:29                 ` Michael Kerrisk (man-pages)
2019-09-24 19:43   ` Michael Kerrisk (man-pages)
2019-09-25  1:48   ` Jann Horn
2019-09-23 11:31 ` Daniel Colascione
2019-09-24 19:42   ` Michael Kerrisk (man-pages)
2019-09-23 14:29 ` Christian Brauner
2019-09-23 20:27   ` Michael Kerrisk (man-pages)
2019-09-23 21:27 ` Eric W. Biederman
2019-09-24 19:10   ` Michael Kerrisk (man-pages)

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=20190924215316.gxev2anuqffegocw@wittgenstein \
    --to=christian.brauner@ubuntu.com \
    --cc=dancol@google.com \
    --cc=ebiederm@xmission.com \
    --cc=fw@deneb.enyo.de \
    --cc=jannh@google.com \
    --cc=joel@joelfernandes.org \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-man@vger.kernel.org \
    --cc=mtk.manpages@gmail.com \
    --cc=oleg@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 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 NNTP newsgroup(s).