linux-kernel - 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]

Date:   Tue, 7 Jul 2020 14:22:20 +0200
From:   Christian Brauner <christian.brauner@...ntu.com>
To:     Kees Cook <keescook@...omium.org>, Christoph Hellwig <hch@....de>
Cc:     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 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?

Thanks!
Christian