From: Christian Brauner <christian@brauner.io>
To: Jann Horn <jannh@google.com>
Cc: Andy Lutomirski <luto@kernel.org>,
David Howells <dhowells@redhat.com>,
"Serge E. Hallyn" <serge@hallyn.com>,
Linux API <linux-api@vger.kernel.org>,
kernel list <linux-kernel@vger.kernel.org>,
Arnd Bergmann <arnd@arndb.de>,
"Eric W. Biederman" <ebiederm@xmission.com>,
Konstantin Khlebnikov <khlebnikov@yandex-team.ru>,
Kees Cook <keescook@chromium.org>,
Alexey Dobriyan <adobriyan@gmail.com>,
Thomas Gleixner <tglx@linutronix.de>,
Michael Kerrisk-manpages <mtk.manpages@gmail.com>,
Jonathan Kowalski <bl0pbl33p@gmail.com>,
"Dmitry V. Levin" <ldv@altlinux.org>,
Andrew Morton <akpm@linux-foundation.org>,
Oleg Nesterov <oleg@redhat.com>,
Nagarathnam Muthusamy <nagarathnam.muthusamy@oracle.com>,
Aleksa Sarai <cyphar@cyphar.com>,
Al Viro <viro@zeniv.linux.org.uk>,
"Joel Fernandes (Google)" <joel@joelfernandes.org>,
Daniel Colascione <dancol@google.com>
Subject: Re: [PATCH v2 2/5] pid: add pidfd_open()
Date: Sat, 30 Mar 2019 00:55:22 +0100 [thread overview]
Message-ID: <20190329235521.wrtuyoualsvcgz5k@brauner.io> (raw)
In-Reply-To: <CAG48ez0Arzdtjxj9zVVTaL=T6tX16e6OcR75rtk9mavCbnMK6w@mail.gmail.com>
On Sat, Mar 30, 2019 at 12:45:46AM +0100, Jann Horn wrote:
> On Fri, Mar 29, 2019 at 4:54 PM Christian Brauner <christian@brauner.io> wrote:
> > /* Introduction */
> > This adds the pidfd_open() syscall.
> > pidfd_open() allows to retrieve file descriptors for a given pid. This
> > includes both file descriptors for processes and file descriptors for
> > threads.
>
> Looks good to me, overall. Apart from a few nits below:
> Reviewed-by: Jann Horn <jannh@google.com>
Thanks! Will fixup the nits and add your Reviewed-by!
>
> [...]
> > diff --git a/kernel/pid.c b/kernel/pid.c
> > index 20881598bdfa..8c9e15e0e463 100644
> > --- a/kernel/pid.c
> > +++ b/kernel/pid.c
> [...]
> > +static struct file *pidfd_open_proc_pid(const struct file *procf, pid_t pid,
> > + const struct pid *pidfd_pid)
> > +{
> > + char name[12]; /* int to strlen + \0 but with */
>
> nit: comment suddenly ends at "but with"?
Will fix.
>
> [...]
> > +}
> > +
> > +static inline int pidfd_to_procfd(int procfd, struct file *pidfd_file)
> > +{
> > + long fd;
>
> nit: This should probably be an int?
Yes.
>
> [...]
> > + return fd;
> > +}
> [...]
> > +static long pidfd_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
> > +{
> > + int procfd = arg;
>
> nit: I think it'd be semantically cleaner to move this assignment into
> the switch case, but I don't feel about it strongly.
Agreed.
>
> > + switch (cmd) {
> > + case PIDFD_GET_PROCFD:
> > + return pidfd_to_procfd(procfd, file);
> > + default:
> > + return -ENOTTY;
> > + }
> > +}
next prev parent reply other threads:[~2019-03-29 23:55 UTC|newest]
Thread overview: 83+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-03-29 15:54 [PATCH v2 0/5] pid: add pidfd_open() Christian Brauner
2019-03-29 15:54 ` [PATCH v2 1/5] Make anon_inodes unconditional Christian Brauner
2019-03-29 15:54 ` [PATCH v2 2/5] pid: add pidfd_open() Christian Brauner
2019-03-29 23:45 ` Jann Horn
2019-03-29 23:55 ` Christian Brauner [this message]
2019-03-30 11:53 ` Jürg Billeter
2019-03-30 14:37 ` Christian Brauner
2019-03-30 14:51 ` Jonathan Kowalski
2019-03-29 15:54 ` [PATCH v2 3/5] signal: support pidfd_open() with pidfd_send_signal() Christian Brauner
2019-03-29 15:54 ` [PATCH v2 4/5] signal: PIDFD_SIGNAL_TID threads via pidfds Christian Brauner
2019-03-30 1:06 ` Jann Horn
2019-03-30 1:22 ` Christian Brauner
2019-03-30 1:34 ` Christian Brauner
2019-03-30 1:42 ` Christian Brauner
2019-03-29 15:54 ` [PATCH v2 5/5] tests: add pidfd_open() tests Christian Brauner
2019-03-30 16:09 ` [PATCH v2 0/5] pid: add pidfd_open() Linus Torvalds
2019-03-30 16:11 ` Daniel Colascione
2019-03-30 16:16 ` Linus Torvalds
2019-03-30 16:18 ` Linus Torvalds
2019-03-31 1:07 ` Joel Fernandes
2019-03-31 2:34 ` Jann Horn
2019-03-31 4:08 ` Joel Fernandes
2019-03-31 4:46 ` Jann Horn
2019-03-31 14:52 ` Linus Torvalds
2019-03-31 15:05 ` Christian Brauner
2019-03-31 15:21 ` Daniel Colascione
2019-03-31 15:33 ` Jonathan Kowalski
2019-03-30 16:19 ` Christian Brauner
2019-03-30 16:24 ` Linus Torvalds
2019-03-30 16:34 ` Daniel Colascione
2019-03-30 16:38 ` Christian Brauner
2019-03-30 17:04 ` Linus Torvalds
2019-03-30 17:12 ` Christian Brauner
2019-03-30 17:24 ` Linus Torvalds
2019-03-30 17:37 ` Christian Brauner
2019-03-30 17:50 ` Jonathan Kowalski
2019-03-30 17:52 ` Christian Brauner
2019-03-30 17:59 ` Jonathan Kowalski
2019-03-30 18:02 ` Christian Brauner
2019-03-30 18:00 ` Jann Horn
2019-03-31 20:09 ` Andy Lutomirski
2019-03-31 21:03 ` Linus Torvalds
2019-03-31 21:10 ` Christian Brauner
2019-03-31 21:17 ` Linus Torvalds
2019-03-31 22:03 ` Christian Brauner
2019-03-31 22:16 ` Linus Torvalds
2019-03-31 22:33 ` Christian Brauner
2019-04-01 0:52 ` Jann Horn
2019-04-01 8:47 ` Yann Droneaud
2019-04-01 10:03 ` Jonathan Kowalski
2019-03-31 23:40 ` Linus Torvalds
2019-04-01 0:09 ` Al Viro
2019-04-01 0:18 ` Linus Torvalds
2019-04-01 0:21 ` Christian Brauner
2019-04-01 6:37 ` Al Viro
2019-04-01 6:41 ` Al Viro
2019-03-31 22:03 ` Jonathan Kowalski
2019-04-01 2:13 ` Andy Lutomirski
2019-04-01 11:40 ` Aleksa Sarai
2019-04-01 15:36 ` Linus Torvalds
2019-04-01 15:47 ` Christian Brauner
2019-04-01 15:55 ` Daniel Colascione
2019-04-01 16:01 ` Linus Torvalds
2019-04-01 16:13 ` Daniel Colascione
2019-04-01 19:42 ` Christian Brauner
2019-04-01 21:30 ` Linus Torvalds
2019-04-01 21:58 ` Jonathan Kowalski
2019-04-01 22:13 ` Linus Torvalds
2019-04-01 22:34 ` Daniel Colascione
2019-04-01 16:07 ` Jonathan Kowalski
2019-04-01 16:15 ` Linus Torvalds
2019-04-01 16:27 ` Jonathan Kowalski
2019-04-01 16:21 ` Daniel Colascione
2019-04-01 16:29 ` Linus Torvalds
2019-04-01 16:45 ` Daniel Colascione
2019-04-01 17:00 ` David Laight
2019-04-01 17:32 ` Linus Torvalds
2019-04-02 11:03 ` Florian Weimer
2019-04-01 16:10 ` Andy Lutomirski
2019-04-01 12:04 ` Christian Brauner
2019-04-01 13:43 ` Jann Horn
2019-03-31 21:19 ` Christian Brauner
2019-03-30 16:37 ` 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=20190329235521.wrtuyoualsvcgz5k@brauner.io \
--to=christian@brauner.io \
--cc=adobriyan@gmail.com \
--cc=akpm@linux-foundation.org \
--cc=arnd@arndb.de \
--cc=bl0pbl33p@gmail.com \
--cc=cyphar@cyphar.com \
--cc=dancol@google.com \
--cc=dhowells@redhat.com \
--cc=ebiederm@xmission.com \
--cc=jannh@google.com \
--cc=joel@joelfernandes.org \
--cc=keescook@chromium.org \
--cc=khlebnikov@yandex-team.ru \
--cc=ldv@altlinux.org \
--cc=linux-api@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=luto@kernel.org \
--cc=mtk.manpages@gmail.com \
--cc=nagarathnam.muthusamy@oracle.com \
--cc=oleg@redhat.com \
--cc=serge@hallyn.com \
--cc=tglx@linutronix.de \
--cc=viro@zeniv.linux.org.uk \
/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).