netdev - Re: [PATCH v6 4/7] pidfd: Replace open-coded partial receive

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <202007081649.622969AAFB@keescook>
Date:   Wed, 8 Jul 2020 16:49:57 -0700
From:   Kees Cook <keescook@...omium.org>
To:     Christian Brauner <christian.brauner@...ntu.com>
Cc:     Christoph Hellwig <hch@....de>, linux-kernel@...r.kernel.org,
        Sargun Dhillon <sargun@...gun.me>,
        Christian Brauner <christian@...uner.io>,
        Tycho Andersen <tycho@...ho.ws>,
        David Laight <David.Laight@...LAB.COM>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Alexander Viro <viro@...iv.linux.org.uk>,
        Aleksa Sarai <cyphar@...har.com>,
        Matt Denton <mpdenton@...gle.com>,
        Jann Horn <jannh@...gle.com>, Chris Palmer <palmer@...gle.com>,
        Robert Sesek <rsesek@...gle.com>,
        Giuseppe Scrivano <gscrivan@...hat.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Andy Lutomirski <luto@...capital.net>,
        Will Drewry <wad@...omium.org>, Shuah Khan <shuah@...nel.org>,
        netdev@...r.kernel.org, containers@...ts.linux-foundation.org,
        linux-api@...r.kernel.org, linux-fsdevel@...r.kernel.org,
        linux-kselftest@...r.kernel.org
Subject: Re: [PATCH v6 4/7] pidfd: Replace open-coded partial receive_fd()

On Tue, Jul 07, 2020 at 02:22:20PM +0200, Christian Brauner wrote:
> On Mon, Jul 06, 2020 at 01:17:17PM -0700, Kees Cook wrote:
> > The sock counting (sock_update_netprioidx() and sock_update_classid()) was
> > missing from pidfd's implementation of received fd installation. Replace
> > the open-coded version with a call to the new receive_fd()
> > helper.
> > 
> > Thanks to Vamshi K Sthambamkadi <vamshi.k.sthambamkadi@...il.com> for
> > catching a missed fput() in an earlier version of this patch.
> > 
> > Fixes: 8649c322f75c ("pid: Implement pidfd_getfd syscall")
> > Reviewed-by: Sargun Dhillon <sargun@...gun.me>
> > Signed-off-by: Kees Cook <keescook@...omium.org>
> > ---
> 
> Thanks!
> Acked-by: Christian Brauner <christian.brauner@...ntu.com>
> 
> Christoph, Kees,
> 
> 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?

I was thinking of just tossing the entire series (hch's and mine) at
-stable since it's relatively narrow. I'll look at what's needed for
backports...

> 
> Thanks!
> Christian

-- 
Kees Cook