All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kees Cook <keescook@chromium.org>
To: Christian Brauner <christian.brauner@ubuntu.com>
Cc: Christoph Hellwig <hch@lst.de>,
	linux-kernel@vger.kernel.org, Sargun Dhillon <sargun@sargun.me>,
	Christian Brauner <christian@brauner.io>,
	Tycho Andersen <tycho@tycho.ws>,
	David Laight <David.Laight@ACULAB.COM>,
	"David S. Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	Aleksa Sarai <cyphar@cyphar.com>,
	Matt Denton <mpdenton@google.com>, Jann Horn <jannh@google.com>,
	Chris Palmer <palmer@google.com>,
	Robert Sesek <rsesek@google.com>,
	Giuseppe Scrivano <gscrivan@redhat.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Andy Lutomirski <luto@amacapital.net>,
	Will Drewry <wad@chromium.org>, Shuah Khan <shuah@kernel.org>,
	netdev@vger.kernel.org, containers@lists.linux-foundation.org,
	linux-api@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	linux-kselftest@vger.kernel.org
Subject: Re: [PATCH v6 4/7] pidfd: Replace open-coded partial receive_fd()
Date: Wed, 8 Jul 2020 23:35:39 -0700	[thread overview]
Message-ID: <202007082330.6B423FE@keescook> (raw)
In-Reply-To: <20200707122220.cazzek4655gj4tj7@wittgenstein>

On Tue, Jul 07, 2020 at 02:22:20PM +0200, Christian Brauner wrote:
> So while the patch is correct it leaves 5.6 and 5.7 with a bug in the
> pidfd_getfd() implementation and that just doesn't seem right. I'm
> wondering whether we should introduce:
> 
> void sock_update(struct file *file)
> {
> 	struct socket *sock;
> 	int error;
> 
> 	sock = sock_from_file(file, &error);
> 	if (sock) {
> 		sock_update_netprioidx(&sock->sk->sk_cgrp_data);
> 		sock_update_classid(&sock->sk->sk_cgrp_data);
> 	}
> }
> 
> and switch pidfd_getfd() over to:
> 
> diff --git a/kernel/pid.c b/kernel/pid.c
> index f1496b757162..c26bba822be3 100644
> --- a/kernel/pid.c
> +++ b/kernel/pid.c
> @@ -642,10 +642,12 @@ static int pidfd_getfd(struct pid *pid, int fd)
>         }
> 
>         ret = get_unused_fd_flags(O_CLOEXEC);
> -       if (ret < 0)
> +       if (ret < 0) {
>                 fput(file);
> -       else
> +       } else {
> +               sock_update(file);
>                 fd_install(ret, file);
> +       }
> 
>         return ret;
>  }
> 
> first thing in the series and then all of the other patches on top of it
> so that we can Cc stable for this and that can get it backported to 5.6,
> 5.7, and 5.8.
> 
> Alternatively, I can make this a separate bugfix patch series which I'll
> send upstream soonish. Or we have specific patches just for 5.6, 5.7,
> and 5.8. Thoughts?

Okay, I looked at hch's clean-ups again and I'm reminded why they
don't make great -stable material. :) The compat bug (also missing the
sock_update()) needs a similar fix (going back to 3.6...), so, yeah,
for ease of backport, probably an explicit sock_update() implementation
(with compat and native scm using it), and a second patch for pidfd.

Let me see what I looks best...

-- 
Kees Cook

  parent reply	other threads:[~2020-07-09  6:35 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-06 20:17 [PATCH v6 0/7] Add seccomp notifier ioctl that enables adding fds Kees Cook
2020-07-06 20:17 ` [PATCH v6 1/7] net/scm: Regularize compat handling of scm_detach_fds() Kees Cook
2020-07-07 11:41   ` Christian Brauner
2020-07-08 23:46     ` Kees Cook
2020-07-06 20:17 ` [PATCH v6 2/7] fs: Move __scm_install_fd() to __receive_fd() Kees Cook
2020-07-07  6:49   ` Christoph Hellwig
2020-07-07 11:45   ` Christian Brauner
2020-07-06 20:17 ` [PATCH v6 3/7] fs: Add receive_fd() wrapper for __receive_fd() Kees Cook
2020-07-07  6:49   ` Christoph Hellwig
2020-07-07 11:49   ` Christian Brauner
2020-07-08 23:48     ` Kees Cook
2020-07-06 20:17 ` [PATCH v6 4/7] pidfd: Replace open-coded partial receive_fd() Kees Cook
2020-07-07 12:22   ` Christian Brauner
2020-07-08 23:49     ` Kees Cook
2020-07-09  6:35     ` Kees Cook [this message]
2020-07-09 12:54       ` Christian Brauner
2020-07-06 20:17 ` [PATCH v6 5/7] fs: Expand __receive_fd() to accept existing fd Kees Cook
2020-07-07 12:38   ` Christian Brauner
2020-07-08 23:52     ` Kees Cook
2020-07-06 20:17 ` [PATCH v6 6/7] seccomp: Introduce addfd ioctl to seccomp user notifier Kees Cook
2020-07-07 13:30   ` Christian Brauner
2020-07-09  6:12     ` Kees Cook
2020-07-09 13:08       ` Christian Brauner
2020-07-09  6:17     ` [PATCH v6.1 " Kees Cook
2020-07-09  6:22       ` Kees Cook
2020-07-06 20:17 ` [PATCH v6 7/7] selftests/seccomp: Test SECCOMP_IOCTL_NOTIF_ADDFD Kees Cook

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=202007082330.6B423FE@keescook \
    --to=keescook@chromium.org \
    --cc=David.Laight@ACULAB.COM \
    --cc=christian.brauner@ubuntu.com \
    --cc=christian@brauner.io \
    --cc=containers@lists.linux-foundation.org \
    --cc=cyphar@cyphar.com \
    --cc=davem@davemloft.net \
    --cc=gregkh@linuxfoundation.org \
    --cc=gscrivan@redhat.com \
    --cc=hch@lst.de \
    --cc=jannh@google.com \
    --cc=kuba@kernel.org \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=luto@amacapital.net \
    --cc=mpdenton@google.com \
    --cc=netdev@vger.kernel.org \
    --cc=palmer@google.com \
    --cc=rsesek@google.com \
    --cc=sargun@sargun.me \
    --cc=shuah@kernel.org \
    --cc=tycho@tycho.ws \
    --cc=viro@zeniv.linux.org.uk \
    --cc=wad@chromium.org \
    /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.